diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 6b379304..e200a9b9 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -1,265 +1,252 @@ # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json -language: en-US -tone_instructions: "Security-first Rust 2024 code review. Flag vulnerabilities, enforce zero-warnings policy, optimize for 10k+ process monitoring. Be direct like a senior security engineer. Focus on privilege separation, performance, and operator reliability." +tone_instructions: >- + Security-first Rust 2024 code review. Flag vulnerabilities, enforce zero-warnings policy, optimize + for 10k+ process monitoring. Be direct like a senior security engineer. Focus on privilege + separation, performance, and operator reliability. early_access: true -enable_free_tier: true +inheritance: true reviews: profile: assertive - request_changes_workflow: true - high_level_summary: true - high_level_summary_placeholder: "@coderabbitai summary" high_level_summary_in_walkthrough: true - auto_title_placeholder: "@coderabbitai" - auto_title_instructions: "Generate PR/MR titles following Conventional Commits format: type(scope): description. Use types: feat, fix, docs, style, refactor, perf, test, build, ci, chore. Use scopes: procmond, daemoneye-agent, daemoneye-cli, daemoneye-lib, security-center, gui, core-feature, process-monitoring, data-models, ipc, database, crypto, async, testing, integration, cross-platform. Keep descriptions concise and action-oriented." - review_status: true - commit_status: true - fail_commit_status: false + auto_title_instructions: >- + Generate PR/MR titles following Conventional Commits format: type(scope): description. Use + types: feat, fix, docs, style, refactor, perf, test, build, ci, chore. Use scopes: procmond, + daemoneye-agent, daemoneye-cli, daemoneye-lib, security-center, gui, core-feature, + process-monitoring, data-models, ipc, database, crypto, async, testing, integration, + cross-platform. Keep descriptions concise and action-oriented. collapse_walkthrough: false - changed_files_summary: true - sequence_diagrams: true - estimate_code_review_effort: true - assess_linked_issues: true - related_issues: true - related_prs: true - suggested_labels: true + labeling_instructions: + - label: rust + instructions: >- + Apply when the PR/MR contains changes to Rust source code files (*.rs). This includes + modifications to any Rust modules, functions, structs, enums, or traits. + - label: core-feature + instructions: >- + Apply when the PR/MR implements or modifies core system functionality including procmond, + daemoneye-agent, daemoneye-cli, or daemoneye-lib components. Focus on fundamental security + monitoring capabilities. + - label: process-monitoring + instructions: >- + Apply when the PR/MR involves process enumeration, monitoring, or collection functionality. + Includes changes to process data structures, collection algorithms, or monitoring + interfaces. + - label: data-models + instructions: >- + Apply when the PR/MR modifies data structures, models, or type definitions. Includes changes + to ProcessRecord, Alert, DetectionRule, or other core data types. + - label: serialization + instructions: >- + Apply when the PR/MR involves serialization/deserialization functionality. Includes changes + to serde implementations, JSON handling, or data format conversions. + - label: ipc + instructions: >- + Apply when the PR/MR involves Inter-Process Communication between procmond, daemoneye-agent, + or daemoneye-cli components. Includes protobuf definitions, Unix sockets, or named pipes. + - label: protobuf + instructions: >- + Apply when the PR/MR involves Protocol Buffer definitions, code generation, or + protobuf-based communication. Includes .proto files or generated protobuf code. + - label: database + instructions: >- + Apply when the PR/MR involves database operations, schema changes, or data storage. Includes + redb, PostgreSQL, or other database-related functionality. + - label: crypto + instructions: >- + Apply when the PR/MR involves cryptographic functionality, hashing, digital signatures, or + security-related cryptographic operations. + - label: async + instructions: >- + Apply when the PR/MR involves asynchronous programming patterns, tokio usage, or async/await + implementations. Includes async functions, futures, or concurrent operations. + - label: testing + instructions: >- + Apply when the PR/MR adds, modifies, or improves test code. Includes unit tests, integration + tests, or test infrastructure changes. + - label: integration + instructions: >- + Apply when the PR/MR involves integration testing, component integration, or cross-component + functionality. Includes end-to-end testing or system integration work. + - label: cross-platform + instructions: >- + Apply when the PR/MR involves multi-platform compatibility features for Linux, macOS, or + Windows. Includes platform-specific code or cross-platform abstractions. + - label: daemoneye-agent + instructions: >- + Apply when the PR/MR specifically involves the daemoneye-agent component. Includes detection + orchestration, alert management, or service lifecycle management. + - label: documentation + instructions: >- + Apply when the PR/MR involves documentation changes, README updates, or code comments. + Includes rustdoc, JSDoc, or markdown documentation. + - label: dependencies + instructions: >- + Apply when the PR/MR updates dependency files like Cargo.toml, package.json, or other + dependency management files. + - label: priority:high + instructions: >- + Apply when the PR/MR addresses high priority issues requiring immediate attention. Use + sparingly for critical security fixes or blocking issues. auto_apply_labels: true - suggested_reviewers: true auto_assign_reviewers: true - poem: true - labeling_instructions: - [ - { - "label": "rust", - "instructions": "Apply when the PR/MR contains changes to Rust source code files (*.rs). This includes modifications to any Rust modules, functions, structs, enums, or traits.", - }, - { - "label": "core-feature", - "instructions": "Apply when the PR/MR implements or modifies core system functionality including procmond, daemoneye-agent, daemoneye-cli, or daemoneye-lib components. Focus on fundamental security monitoring capabilities.", - }, - { - "label": "process-monitoring", - "instructions": "Apply when the PR/MR involves process enumeration, monitoring, or collection functionality. Includes changes to process data structures, collection algorithms, or monitoring interfaces.", - }, - { - "label": "data-models", - "instructions": "Apply when the PR/MR modifies data structures, models, or type definitions. Includes changes to ProcessRecord, Alert, DetectionRule, or other core data types.", - }, - { - "label": "serialization", - "instructions": "Apply when the PR/MR involves serialization/deserialization functionality. Includes changes to serde implementations, JSON handling, or data format conversions.", - }, - { - "label": "ipc", - "instructions": "Apply when the PR/MR involves Inter-Process Communication between procmond, daemoneye-agent, or daemoneye-cli components. Includes protobuf definitions, Unix sockets, or named pipes.", - }, - { - "label": "protobuf", - "instructions": "Apply when the PR/MR involves Protocol Buffer definitions, code generation, or protobuf-based communication. Includes .proto files or generated protobuf code.", - }, - { - "label": "database", - "instructions": "Apply when the PR/MR involves database operations, schema changes, or data storage. Includes redb, PostgreSQL, or other database-related functionality.", - }, - { - "label": "crypto", - "instructions": "Apply when the PR/MR involves cryptographic functionality, hashing, digital signatures, or security-related cryptographic operations.", - }, - { - "label": "async", - "instructions": "Apply when the PR/MR involves asynchronous programming patterns, tokio usage, or async/await implementations. Includes async functions, futures, or concurrent operations.", - }, - { - "label": "testing", - "instructions": "Apply when the PR/MR adds, modifies, or improves test code. Includes unit tests, integration tests, or test infrastructure changes.", - }, - { - "label": "integration", - "instructions": "Apply when the PR/MR involves integration testing, component integration, or cross-component functionality. Includes end-to-end testing or system integration work.", - }, - { - "label": "cross-platform", - "instructions": "Apply when the PR/MR involves multi-platform compatibility features for Linux, macOS, or Windows. Includes platform-specific code or cross-platform abstractions.", - }, - { - "label": "daemoneye-agent", - "instructions": "Apply when the PR/MR specifically involves the daemoneye-agent component. Includes detection orchestration, alert management, or service lifecycle management.", - }, - { - "label": "documentation", - "instructions": "Apply when the PR/MR involves documentation changes, README updates, or code comments. Includes rustdoc, JSDoc, or markdown documentation.", - }, - { - "label": "dependencies", - "instructions": "Apply when the PR/MR updates dependency files like Cargo.toml, package.json, or other dependency management files.", - }, - { - "label": "priority:high", - "instructions": "Apply when the PR/MR addresses high priority issues requiring immediate attention. Use sparingly for critical security fixes or blocking issues.", - }, - ] path_filters: - [ - "procmond/**", - "daemoneye-agent/**", - "daemoneye-cli/**", - "daemoneye-lib/**", - "collector-core/**", - "docs/src/**", - "spec/**", - ".kiro/**/*.md", - ".cursor/**/*.mdc", - ".github/**", - "!.github/workflows/release.yml", - "*.md", - "*.toml", - "*.yaml", - "*.yml", - "*.json", - "*.sh", - "*.py", - "*.js", - "*.ts", - "*.tsx", - "justfile", - "!target/**", - "!dist/**", - "!docs/book/**", - "!node_modules/**", - "!*.woff2", - "!*.png", - "!*.svg", - "!*.ico", - "!*.wxs", - ] + - procmond/** + - daemoneye-agent/** + - daemoneye-cli/** + - daemoneye-lib/** + - collector-core/** + - docs/src/** + - spec/** + - .kiro/**/*.md + - .cursor/**/*.mdc + - .github/** + - "!.github/workflows/release.yml" + - "*.md" + - "*.toml" + - "*.yaml" + - "*.yml" + - "*.json" + - "*.sh" + - "*.py" + - "*.js" + - "*.ts" + - "*.tsx" + - justfile + - "!target/**" + - "!dist/**" + - "!docs/book/**" + - "!node_modules/**" + - "!*.woff2" + - "!*.png" + - "!*.svg" + - "!*.ico" + - "!*.wxs" path_instructions: - [ - { - "path": "procmond/**", - "instructions": "CRITICAL: Privileged process collector with minimal attack surface. Enforce principle of least privilege, immediate privilege dropping after init, no network access, write-only access to audit ledger. Flag any unsafe code, privilege escalation risks, or attack surface expansions. Focus on security, performance, and minimal dependencies. Does not expose any shared code, so any reusable code should be in daemoneye-lib or collector-core.", - }, - { - "path": "daemoneye-agent/**", - "instructions": "User-space orchestrator for detection, alerting, and procmond lifecycle management. Enforce outbound-only network connections, read/write event store access, IPC client patterns. Focus on SQL-to-IPC translation using sqlparser, task generation for procmond, overcollection handling, SQL injection prevention, concurrent alert delivery, service management, and error handling with thiserror/anyhow. Flag performance regressions in detection rule execution.", - }, - { - "path": "daemoneye-cli/**", - "instructions": "Operator CLI interface. Enforce NO direct database access - communicates only through daemoneye-agent. No network access, comprehensive error messages, JSON/table output support. Focus on user experience, shell completions, color handling, and operational efficiency for SOC environments.", - }, - { - "path": "daemoneye-lib/**", - "instructions": "Shared library providing common functionality across all components. Enforce trait-based abstractions, security boundaries, comprehensive error handling. Focus on modularity, testability, and performance. Flag any unsafe code or potential panics. Ensure all public APIs are well-documented with rustdoc comments. Stability and performance of the library is critical.", - }, - { - "path": "collector-core/**", - "instructions": "Shared framework library providing common functionality for collectors, such as procmond. Enforce trait-based abstractions, security boundaries, comprehensive error handling. Focus on modularity, testability, and performance. Flag any unsafe code or potential panics. Ensure all public APIs are well-documented with rustdoc comments. Stability and performance of the library is critical.", - }, - { - "path": "daemoneye-eventbus/**", - "instructions": "Cross-platform IPC event bus for DaemonEye monitoring system. Enforce cross-platform compatibility: PRIMARY platforms (Linux Ubuntu 20.04+ LTS, RHEL/CentOS 8+, Alma/Rocky Linux 8+, Debian 11+ LTS, macOS 14.0+, Windows 10+, Windows Server 2019+, Windows Server 2022, Windows 11) and SECONDARY platforms (Alpine 3.16+, Amazon Linux 2+, Ubuntu 18.04, RHEL 7, macOS 12.0+, FreeBSD 13.0+). Use Windows named pipes and Unix domain sockets. Focus on performance (10k+ messages/sec), security (local IPC only), and reliability (at-most-once delivery). Flag any network exposure, privilege escalation, or performance regressions. Ensure proper error handling and resource cleanup.", - }, - { - "path": "docs/src/**", - "instructions": "Documentation source files. Enforce clear, technical writing for cybersecurity professionals. Focus on accuracy, completeness, and operational relevance. Flag missing security considerations or unclear technical explanations.", - }, - { - "path": "spec/**", - "instructions": "Project specifications and architecture documents. Enforce technical accuracy, architectural consistency, and clear requirements. Focus on security-first design principles and operational constraints.", - }, - { - "path": ".kiro/**", - "instructions": "Project documentation and specifications, written in spec-driven development, used by the Kiro AI assistant. Enforce technical accuracy, architectural consistency, and clear requirements. Focus on security-first design principles and operational constraints.", - }, - { - "path": "*.md", - "instructions": "Project documentation. Enforce clear, professional writing for security operations teams. Focus on accuracy, completeness, and operational relevance. Flag missing security considerations or unclear technical explanations.", - }, - { - "path": "Cargo.toml", - "instructions": "Dependency and workspace configuration. Enforce minimal dependencies, security-focused crate selection, proper version pinning. Flag potential security vulnerabilities in dependencies or workspace configuration issues.", - }, - { - "path": "justfile", - "instructions": "Task runner configuration. Enforce DRY principles, clear command organization, security considerations in build tasks. Focus on operational efficiency and maintainability.", - }, - { - "path": "*.yaml", - "instructions": "YAML configuration files. Enforce proper YAML syntax, consistent indentation, and clear structure. Focus on configuration validation and security considerations for CI/CD workflows.", - }, - { - "path": "*.yml", - "instructions": "YAML configuration files. Enforce proper YAML syntax, consistent indentation, and clear structure. Focus on configuration validation and security considerations for CI/CD workflows.", - }, - { - "path": "*.json", - "instructions": "JSON configuration files. Enforce valid JSON syntax, proper structure, and security considerations. Focus on configuration validation and maintainability.", - }, - { - "path": "*.sh", - "instructions": "Shell scripts. Enforce proper shell scripting practices, error handling, and security considerations. Focus on portability, maintainability, and operational efficiency.", - }, - { - "path": "*.js", - "instructions": "JavaScript files. Enforce modern JavaScript practices, security considerations, and maintainability. Focus on performance and compatibility.", - }, - { - "path": "*.ts", - "instructions": "TypeScript files. Enforce TypeScript best practices, type safety, and security considerations. Focus on maintainability and developer experience.", - }, - { - "path": "*.tsx", - "instructions": "React TypeScript files. Enforce React and TypeScript best practices, component design patterns, and security considerations. Focus on user experience and maintainability.", - }, - ] - abort_on_close: true - disable_cache: false + - path: procmond/** + instructions: >- + CRITICAL: Privileged process collector with minimal attack surface. Enforce principle of + least privilege, immediate privilege dropping after init, no network access, write-only + access to audit ledger. Flag any unsafe code, privilege escalation risks, or attack surface + expansions. Focus on security, performance, and minimal dependencies. Does not expose any + shared code, so any reusable code should be in daemoneye-lib or collector-core. + - path: daemoneye-agent/** + instructions: >- + User-space orchestrator for detection, alerting, and procmond lifecycle management. Enforce + outbound-only network connections, read/write event store access, IPC client patterns. Focus + on SQL-to-IPC translation using sqlparser, task generation for procmond, overcollection + handling, SQL injection prevention, concurrent alert delivery, service management, and error + handling with thiserror/anyhow. Flag performance regressions in detection rule execution. + - path: daemoneye-cli/** + instructions: >- + Operator CLI interface. Enforce NO direct database access - communicates only through + daemoneye-agent. No network access, comprehensive error messages, JSON/table output support. + Focus on user experience, shell completions, color handling, and operational efficiency for + SOC environments. + - path: daemoneye-lib/** + instructions: >- + Shared library providing common functionality across all components. Enforce trait-based + abstractions, security boundaries, comprehensive error handling. Focus on modularity, + testability, and performance. Flag any unsafe code or potential panics. Ensure all public + APIs are well-documented with rustdoc comments. Stability and performance of the library is + critical. + - path: collector-core/** + instructions: >- + Shared framework library providing common functionality for collectors, such as procmond. + Enforce trait-based abstractions, security boundaries, comprehensive error handling. Focus + on modularity, testability, and performance. Flag any unsafe code or potential panics. + Ensure all public APIs are well-documented with rustdoc comments. Stability and performance + of the library is critical. + - path: daemoneye-eventbus/** + instructions: >- + Cross-platform IPC event bus for DaemonEye monitoring system. Enforce cross-platform + compatibility: PRIMARY platforms (Linux Ubuntu 20.04+ LTS, RHEL/CentOS 8+, Alma/Rocky Linux + 8+, Debian 11+ LTS, macOS 14.0+, Windows 10+, Windows Server 2019+, Windows Server 2022, + Windows 11) and SECONDARY platforms (Alpine 3.16+, Amazon Linux 2+, Ubuntu 18.04, RHEL 7, + macOS 12.0+, FreeBSD 13.0+). Use Windows named pipes and Unix domain sockets. Focus on + performance (10k+ messages/sec), security (local IPC only), and reliability (at-most-once + delivery). Flag any network exposure, privilege escalation, or performance regressions. + Ensure proper error handling and resource cleanup. + - path: docs/src/** + instructions: >- + Documentation source files. Enforce clear, technical writing for cybersecurity + professionals. Focus on accuracy, completeness, and operational relevance. Flag missing + security considerations or unclear technical explanations. + - path: spec/** + instructions: >- + Project specifications and architecture documents. Enforce technical accuracy, architectural + consistency, and clear requirements. Focus on security-first design principles and + operational constraints. + - path: .kiro/** + instructions: >- + Project documentation and specifications, written in spec-driven development, used by the + Kiro AI assistant. Enforce technical accuracy, architectural consistency, and clear + requirements. Focus on security-first design principles and operational constraints. + - path: "*.md" + instructions: >- + Project documentation. Enforce clear, professional writing for security operations teams. + Focus on accuracy, completeness, and operational relevance. Flag missing security + considerations or unclear technical explanations. + - path: Cargo.toml + instructions: >- + Dependency and workspace configuration. Enforce minimal dependencies, security-focused crate + selection, proper version pinning. Flag potential security vulnerabilities in dependencies + or workspace configuration issues. + - path: justfile + instructions: >- + Task runner configuration. Enforce DRY principles, clear command organization, security + considerations in build tasks. Focus on operational efficiency and maintainability. + - path: "*.yaml" + instructions: >- + YAML configuration files. Enforce proper YAML syntax, consistent indentation, and clear + structure. Focus on configuration validation and security considerations for CI/CD + workflows. + - path: "*.yml" + instructions: >- + YAML configuration files. Enforce proper YAML syntax, consistent indentation, and clear + structure. Focus on configuration validation and security considerations for CI/CD + workflows. + - path: "*.json" + instructions: >- + JSON configuration files. Enforce valid JSON syntax, proper structure, and security + considerations. Focus on configuration validation and maintainability. + - path: "*.sh" + instructions: >- + Shell scripts. Enforce proper shell scripting practices, error handling, and security + considerations. Focus on portability, maintainability, and operational efficiency. + - path: "*.js" + instructions: >- + JavaScript files. Enforce modern JavaScript practices, security considerations, and + maintainability. Focus on performance and compatibility. + - path: "*.ts" + instructions: >- + TypeScript files. Enforce TypeScript best practices, type safety, and security + considerations. Focus on maintainability and developer experience. + - path: "*.tsx" + instructions: >- + React TypeScript files. Enforce React and TypeScript best practices, component design + patterns, and security considerations. Focus on user experience and maintainability. + slop_detection: + label: slop auto_review: - enabled: true - auto_incremental_review: true - ignore_title_keywords: [] - labels: [] - drafts: false - base_branches: [] - ignore_usernames: [] + base_branches: + - main finishing_touches: - docstrings: - enabled: true unit_tests: + enabled: false + simplify: enabled: true pre_merge_checks: - docstrings: - mode: warning - threshold: 80 title: - mode: warning - requirements: "Must follow Conventional Commits specification: type(scope): description. Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore. Scopes: auth, api, cli, models, detection, alerting, etc. Breaking changes indicated with ! in header or BREAKING CHANGE: in footer." - description: - mode: warning - issue_assessment: - mode: warning - custom_checks: [] + mode: error + requirements: >- + Must follow Conventional Commits specification: type(scope): description. Types: feat, fix, + docs, style, refactor, perf, test, build, ci, chore. Scopes: auth, api, cli, models, + detection, alerting, etc. Breaking changes indicated with ! in header or BREAKING CHANGE: in + footer. tools: - ast-grep: - rule_dirs: [] - util_dirs: [] - essential_rules: true - packages: [] shellcheck: enabled: false ruff: enabled: false - markdownlint: - enabled: true - github-checks: - enabled: true - timeout_ms: 90000 - languagetool: - enabled: true - enabled_rules: [] - disabled_rules: [] - enabled_categories: [] - disabled_categories: [] - enabled_only: false - level: default biome: enabled: false hadolint: @@ -268,167 +255,166 @@ reviews: enabled: false phpstan: enabled: false - level: default phpmd: enabled: false phpcs: enabled: false golangci-lint: enabled: false - yamllint: - enabled: true - gitleaks: - enabled: true - checkov: - enabled: true detekt: enabled: false eslint: enabled: false flake8: enabled: false - rubocop: + fortitudeLint: enabled: false - buf: + rubocop: enabled: false regal: enabled: false - actionlint: - enabled: true pmd: enabled: false + clang: + enabled: false cppcheck: enabled: false - semgrep: - enabled: true circleci: enabled: false - clippy: - enabled: true - sqlfluff: - enabled: true prismaLint: enabled: false pylint: enabled: false - oxc: - enabled: false shopifyThemeCheck: enabled: false luacheck: enabled: false brakeman: enabled: false - dotenvLint: - enabled: true htmlhint: enabled: false - checkmake: - enabled: true - osvScanner: - enabled: true + stylelint: + enabled: false + smartyLint: + enabled: false + emberTemplateLint: + enabled: false chat: - art: true - auto_reply: true + allow_non_org_members: false integrations: jira: - usage: disabled - linear: - usage: auto + usage: enabled knowledge_base: - opt_out: false - web_search: - enabled: true code_guidelines: - enabled: true - filePatterns: [] - learnings: - scope: auto + filePatterns: + - spec/**/*.md issues: scope: local jira: - usage: disabled - project_keys: [] - linear: - usage: auto - team_keys: [] + usage: enabled + project_keys: + - END pull_requests: scope: local mcp: usage: enabled - disabled_servers: [] code_generation: docstrings: - language: en-US - path_instructions: [ - { - "path": "**/*.rs", - "instructions": "Generate rustdoc comments following standard format: /// for public items, //! for module docs. Include brief description, parameter docs with # Arguments, return docs with # Returns, error docs with # Errors, and examples with # Examples. Use ```rust for runnable examples, ```rust,no_run for non-runnable. Focus on security implications and performance characteristics.", - }, - { - "path": "daemoneye-cli/**", - "instructions": "Generate rustdoc comments following standard format with CLI focus: /// for public items, //! for module docs. Include # Arguments, # Returns, # Errors, and # Examples sections. Focus on operator workflows, error messages, and output formatting. Use ```rust,no_run for security-sensitive CLI examples.", - }, - { - "path": "daemoneye-lib/**", - "instructions": "Generate rustdoc comments following standard format with shared library focus: /// for public items, //! for module docs. Include # Arguments, # Returns, # Errors, and # Examples sections. Emphasize trait-based abstractions, security boundaries, and cross-component usage. Use ```rust for runnable examples.", - }, - # Not yet implemented - { - "path": "gui/**", - "instructions": "Generate JSDoc comments following TypeScript/React format: /** */ for components and functions. Include @param, @returns, @example, and @since tags. Focus on user experience, real-time data handling, and security operations workflows. Use ```tsx for component examples.", - }, - ] + path_instructions: + - path: "**/*.rs" + instructions: >- + Generate rustdoc comments following standard format: /// for public items, //! for module + docs. Include brief description, parameter docs with # Arguments, return docs with # + Returns, error docs with # Errors, and examples with # Examples. Use ```rust for runnable + examples, ```rust,no_run for non-runnable. Focus on security implications and performance + characteristics. + - path: daemoneye-cli/** + instructions: >- + Generate rustdoc comments following standard format with CLI focus: /// for public items, + //! for module docs. Include # Arguments, # Returns, # Errors, and # Examples sections. + Focus on operator workflows, error messages, and output formatting. Use ```rust,no_run for + security-sensitive CLI examples. + - path: daemoneye-lib/** + instructions: >- + Generate rustdoc comments following standard format with shared library focus: /// for + public items, //! for module docs. Include # Arguments, # Returns, # Errors, and # + Examples sections. Emphasize trait-based abstractions, security boundaries, and + cross-component usage. Use ```rust for runnable examples. + - path: gui/** + instructions: >- + Generate JSDoc comments following TypeScript/React format: /** */ for components and + functions. Include @param, @returns, @example, and @since tags. Focus on user experience, + real-time data handling, and security operations workflows. Use ```tsx for component + examples. unit_tests: - path_instructions: [ - { - "path": "**/*.rs", - "instructions": "Generate unit tests using standard Rust testing patterns: #[cfg(test)] mod tests, #[tokio::test] for async, use super::*. Focus on security boundaries, error handling, and performance. Use mock dependencies, insta for snapshot testing, and predicates for validation. Test both success and failure paths.", - }, - { - "path": "procmond/**", - "instructions": "Generate security-focused unit tests for privileged process collector. Test privilege escalation scenarios, privilege dropping behavior, and attack surface boundaries. Use mock system calls and test error conditions thoroughly.", - }, - { - "path": "daemoneye-cli/**", - "instructions": "Generate CLI-focused unit tests using insta for snapshot testing and predicates for validation. Test JSON/table output formatting, shell completions, error messages, and user experience. Test both interactive and non-interactive modes.", - }, - # Not yet implemented - { - "path": "gui/**", - "instructions": "Generate React/TypeScript unit tests using Jest and React Testing Library. Test component rendering, user interactions, state management, and API integration. Focus on accessibility and responsive design testing.", - }, - ] + path_instructions: + - path: "**/*.rs" + instructions: >- + Generate unit tests using standard Rust testing patterns: #[cfg(test)] mod tests, + #[tokio::test] for async, use super::*. Focus on security boundaries, error handling, and + performance. Use mock dependencies, insta for snapshot testing, and predicates for + validation. Test both success and failure paths. + - path: procmond/** + instructions: >- + Generate security-focused unit tests for privileged process collector. Test privilege + escalation scenarios, privilege dropping behavior, and attack surface boundaries. Use mock + system calls and test error conditions thoroughly. + - path: daemoneye-cli/** + instructions: >- + Generate CLI-focused unit tests using insta for snapshot testing and predicates for + validation. Test JSON/table output formatting, shell completions, error messages, and user + experience. Test both interactive and non-interactive modes. + - path: gui/** + instructions: >- + Generate React/TypeScript unit tests using Jest and React Testing Library. Test component + rendering, user interactions, state management, and API integration. Focus on + accessibility and responsive design testing. issue_enrichment: auto_enrich: enabled: true - planning: - enabled: true auto_planning: enabled: false labels: - - "enhancement" - - "bug" - + - enhancement + - bug labeling: - auto_apply_labels: true labeling_instructions: - - label: "bug" - instructions: "Apply when the issue reports something that isn't working correctly. Look for error messages, unexpected behavior, crashes, or regressions in existing functionality." - - label: "enhancement" - instructions: "Apply when the issue requests new features or improvements. This includes new CLI options, new LLM providers, new output formats, performance improvements, or usability enhancements." - - label: "documentation" - instructions: "Apply when the issue is about missing, incorrect, or unclear documentation. This includes README updates, API documentation, examples, or inline code comments." - - label: "good first issue" - instructions: "Apply when the issue is well-scoped, has clear requirements, and doesn't require deep knowledge of the codebase. Good for newcomers to contribute." - - label: "help wanted" - instructions: "Apply when the issue needs community input, additional expertise, or the maintainers explicitly request assistance." - - label: "question" - instructions: "Apply when the issue is asking for clarification, guidance, or discussion rather than reporting a bug or requesting a feature." - - label: "duplicate" - instructions: "Apply when this issue duplicates an existing open or recently closed issue. Reference the original issue." - - label: "invalid" - instructions: "Apply when the issue doesn't provide enough information, is not related to this project, or cannot be reproduced." - - label: "wontfix" - instructions: "Apply when the issue describes behavior that is working as intended, is out of scope for the project, or conflicts with project goals." + - label: bug + instructions: >- + Apply when the issue reports something that isn't working correctly. Look for error + messages, unexpected behavior, crashes, or regressions in existing functionality. + - label: enhancement + instructions: >- + Apply when the issue requests new features or improvements. This includes new CLI options, + new LLM providers, new output formats, performance improvements, or usability + enhancements. + - label: documentation + instructions: >- + Apply when the issue is about missing, incorrect, or unclear documentation. This includes + README updates, API documentation, examples, or inline code comments. + - label: good first issue + instructions: >- + Apply when the issue is well-scoped, has clear requirements, and doesn't require deep + knowledge of the codebase. Good for newcomers to contribute. + - label: help wanted + instructions: >- + Apply when the issue needs community input, additional expertise, or the maintainers + explicitly request assistance. + - label: question + instructions: >- + Apply when the issue is asking for clarification, guidance, or discussion rather than + reporting a bug or requesting a feature. + - label: duplicate + instructions: >- + Apply when this issue duplicates an existing open or recently closed issue. Reference the + original issue. + - label: invalid + instructions: >- + Apply when the issue doesn't provide enough information, is not related to this project, + or cannot be reproduced. + - label: wontfix + instructions: >- + Apply when the issue describes behavior that is working as intended, is out of scope for + the project, or conflicts with project goals. + auto_apply_labels: true diff --git a/.gitignore b/.gitignore index e373fafc..f9fe897f 100644 --- a/.gitignore +++ b/.gitignore @@ -141,3 +141,4 @@ docs/plans .tessl/tiles/ **/*.local.* .context/**/*.md +todos/ diff --git a/.tessl/RULES.md b/.tessl/RULES.md index bdec5bdf..28690dc5 100644 --- a/.tessl/RULES.md +++ b/.tessl/RULES.md @@ -6,10 +6,6 @@ This file is updated when running `tessl install`. If a linked file is missing, @tiles/popey/github-action-monitor/rules/post-push-monitor.md [post-push-monitor](tiles/popey/github-action-monitor/rules/post-push-monitor.md) -## tessl-labs/good-oss-citizen — good-oss-citizen - -@tiles/tessl-labs/good-oss-citizen/rules/good-oss-citizen.md [good-oss-citizen](tiles/tessl-labs/good-oss-citizen/rules/good-oss-citizen.md) - ## tessl-labs/skill-discovery — skill-discovery-rules @tiles/tessl-labs/skill-discovery/rules/skill-discovery-rules.md [skill-discovery-rules](tiles/tessl-labs/skill-discovery/rules/skill-discovery-rules.md) diff --git a/Cargo.lock b/Cargo.lock index b7a5aa58..54c0e157 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,6 +2,19 @@ # It is not intended for manual editing. version = 4 +[[package]] +name = "ahash" +version = "0.8.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" +dependencies = [ + "cfg-if", + "getrandom 0.3.4", + "once_cell", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.4" @@ -20,6 +33,12 @@ dependencies = [ "cc", ] +[[package]] +name = "ambient-authority" +version = "0.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9d4ee0d472d1cd2e28c97dfa124b3d8d992e10eb0a035f33f5d12e3a177ba3b" + [[package]] name = "android_system_properties" version = "0.1.5" @@ -244,6 +263,48 @@ version = "1.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e629a66d692cb9ff1a1c664e41771b3dcaf961985a9774c0eb0bd1b51cf60a48" +[[package]] +name = "cap-fs-ext" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d78e5a3368ae89b7cb68186411452b4b9fac8b41be9c19bf3f47c2d2c8e36e6b" +dependencies = [ + "cap-primitives", + "cap-std", + "io-lifetimes 3.0.1", + "windows-sys 0.61.2", +] + +[[package]] +name = "cap-primitives" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cdadbd7c002d3a484b35243669abdae85a0ebaded5a61117169dc3400f9a7ff0" +dependencies = [ + "ambient-authority", + "fs-set-times", + "io-extras", + "io-lifetimes 3.0.1", + "ipnet", + "maybe-owned", + "rustix", + "rustix-linux-procfs", + "windows-sys 0.61.2", + "winx", +] + +[[package]] +name = "cap-std" +version = "4.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7281235d6e96d3544ca18bba9049be92f4190f8d923e3caef1b5f66cfa752608" +dependencies = [ + "cap-primitives", + "io-extras", + "io-lifetimes 3.0.1", + "rustix", +] + [[package]] name = "cast" version = "0.3.0" @@ -380,6 +441,8 @@ dependencies = [ "anyhow", "async-trait", "bitflags", + "cap-fs-ext", + "cap-std", "criterion", "crossbeam", "daemoneye-eventbus", @@ -679,12 +742,14 @@ dependencies = [ "prost", "prost-build", "prost-types", + "quick_cache", "rand 0.10.0", "redb", "rs_merkle", "serde", "serde_json", "sha2 0.11.0", + "sha3", "sqlparser", "sysinfo", "tempfile", @@ -867,6 +932,17 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" +[[package]] +name = "fs-set-times" +version = "0.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94e7099f6313ecacbe1256e8ff9d617b75d1bcb16a6fddef94866d225a01a14a" +dependencies = [ + "io-lifetimes 2.0.4", + "rustix", + "windows-sys 0.59.0", +] + [[package]] name = "futures" version = "0.3.32" @@ -1132,6 +1208,34 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "io-extras" +version = "0.19.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20fd6de4ccfcc187e38bc21cfa543cb5a302cb86a8b114eb7f0bf0dc9f8ac00f" +dependencies = [ + "io-lifetimes 3.0.1", + "windows-sys 0.59.0", +] + +[[package]] +name = "io-lifetimes" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06432fb54d3be7964ecd3649233cddf80db2832f47fec34c01f65b3d9d774983" + +[[package]] +name = "io-lifetimes" +version = "3.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2f0fb0570afe1fed943c5c3d4102d5358592d8625fda6a0007fdbe65a92fba96" + +[[package]] +name = "ipnet" +version = "2.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -1172,6 +1276,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "keccak" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e24a010dd405bd7ed803e5253182815b41bf2e6a80cc3bfc066658e03a198aa" +dependencies = [ + "cfg-if", + "cpufeatures 0.3.0", +] + [[package]] name = "lazy_static" version = "1.5.0" @@ -1232,6 +1346,12 @@ dependencies = [ "regex-automata", ] +[[package]] +name = "maybe-owned" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4facc753ae494aeb6e3c22f839b158aebd4f9270f55cd3c79906c45476c47ab4" + [[package]] name = "memchr" version = "2.8.0" @@ -1575,7 +1695,9 @@ dependencies = [ "criterion", "daemoneye-eventbus", "daemoneye-lib", + "futures", "insta", + "libc", "postcard", "predicates", "proptest", @@ -1684,6 +1806,18 @@ version = "1.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" +[[package]] +name = "quick_cache" +version = "0.6.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a70b1b8b47e31d0498ecbc3c5470bb931399a8bfed1fd79d1717a61ce7f96e3" +dependencies = [ + "ahash", + "equivalent", + "hashbrown 0.16.1", + "parking_lot", +] + [[package]] name = "quote" version = "1.0.45" @@ -1904,6 +2038,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "rustix-linux-procfs" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2fc84bf7e9aa16c4f2c758f27412dc9841341e16aa682d9c7ac308fe3ee12056" +dependencies = [ + "once_cell", + "rustix", +] + [[package]] name = "rustversion" version = "1.0.22" @@ -2049,6 +2193,16 @@ dependencies = [ "digest 0.11.2", ] +[[package]] +name = "sha3" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "be176f1a57ce4e3d31c1a166222d9768de5954f811601fb7ca06fc8203905ce1" +dependencies = [ + "digest 0.11.2", + "keccak", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -2216,9 +2370,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.51.0" +version = "1.51.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bd1c4c0fc4a7ab90fc15ef6daaa3ec3b893f004f915f2392557ed23237820cd" +checksum = "f66bf9585cda4b724d3e78ab34b73fb2bbaba9011b9bfdf69dc836382ea13b8c" dependencies = [ "bytes", "libc", @@ -2963,6 +3117,16 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "winx" +version = "0.36.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f3fd376f71958b862e7afb20cfe5a22830e1963462f3a17f49d82a6c1d1f42d" +dependencies = [ + "bitflags", + "windows-sys 0.59.0", +] + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/Cargo.toml b/Cargo.toml index 6423520c..44f3473c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,6 +56,10 @@ blake3 = { version = "1.8.4", default-features = false, features = ["std"] } # Buffer and byte utilities bytes = "1.11.1" +# Capability-based filesystem access (TOCTOU-safe opens) +cap-fs-ext = "=4.0.2" +cap-std = "=4.0.2" + # Date/time and utilities chrono = { version = "0.4.44", features = ["serde"] } @@ -83,6 +87,10 @@ futures-util = "0.3.32" hostname-validator = "1.1.1" insta = { version = "1.47.2", features = ["filters"] } interprocess = { version = "2.4.0", features = ["tokio"] } +# Exact-pinned: libc exposes raw OS constants (O_NOFOLLOW, ELOOP, etc.) +# that security-critical code in procmond relies on. See AGENTS.md +# "Pin security-critical deps". +libc = "=0.2.184" parking_lot = "0.12.5" # Serialization @@ -96,6 +104,12 @@ proptest = "1.11.0" prost = "0.14.3" prost-build = "0.14.3" prost-types = "0.14.3" + +# Bounded in-memory cache (hash result store). +# Exact-pinned per AGENTS.md "Pin security-critical deps" — this cache +# holds authoritative hash results so a silent upstream change here +# could affect integrity decisions. +quick_cache = "=0.6.21" rand = "0.10.0" # Database and storage @@ -109,12 +123,18 @@ security-framework = "3.7.0" serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.149" sha2 = "0.11.0" +# Exact-pinned: SHA-3 is a cryptographic primitive and must not change +# silently under a caret update. See AGENTS.md "Pin security-critical deps". +sha3 = "=0.11.0" sqlparser = "0.61.0" sysinfo = "0.38.4" tempfile = "3.27.0" thiserror = "2.0.18" -tokio = { version = "1.51.0", features = [ +# Exact-pinned: Tokio is the async runtime underpinning privilege +# separation, IPC, and agent coordination. See AGENTS.md "Pin +# security-critical deps". +tokio = { version = "=1.51.1", features = [ "rt", "rt-multi-thread", "net", diff --git a/SECURITY_AUDIT_2026-04-03.md b/SECURITY_AUDIT_2026-04-03.md deleted file mode 100644 index cd60a630..00000000 --- a/SECURITY_AUDIT_2026-04-03.md +++ /dev/null @@ -1,490 +0,0 @@ -# DaemonEye Security Audit Report - -**Date**: 2026-04-03 **Scope**: Full workspace -- 6 crates (collector-core, daemoneye-agent, daemoneye-cli, daemoneye-eventbus, daemoneye-lib, procmond) **Branch**: `todo_cleanups` at commit `16db9f4` **Classification**: READ-ONLY research audit - ---- - -## Executive Summary - -DaemonEye demonstrates strong security foundations: `unsafe_code = "forbid"` at the workspace level, comprehensive clippy lint configuration, overflow checks in all profiles, pinned action SHAs in the primary CI workflow, CRC32 integrity validation on IPC frames, and AST-based SQL validation for detection rules. The project uses `cargo deny` and `cargo audit` for dependency supply chain hardening. - -However, the audit identified **3 Critical**, **5 High**, **6 Medium**, and **4 Low** severity findings. The most urgent issues are: a WAL durability gap that can lose audit trail entries on crash, a UTF-8 panic in the correlation metadata constructor, and a privilege separation violation where the CLI can write to the database despite the architecture mandating read-only access. - ---- - -## Findings - -### CRITICAL-01: WAL Missing fsync -- Audit Trail Durability Gap - -| Attribute | Value | -| ------------ | ----------------------------------------------------------------------------------------------------- | -| **Severity** | Critical | -| **CWE** | CWE-311 (Missing Encryption of Sensitive Data), CWE-755 (Improper Handling of Exceptional Conditions) | -| **File** | `procmond/src/wal.rs`, lines 558-619 | - -**Description**: The `WriteAheadLog::write()` and `write_with_type()` methods never call `flush()` or `sync_data()` on the file handle after writing entries. Tokio's `AsyncWriteExt::write_all()` writes to the OS buffer but does not guarantee persistence to disk. On system crash or power loss, WAL entries that have been "written" but not fsynced will be lost silently. - -**Attack Scenario**: An attacker who can cause a process crash (e.g., via resource exhaustion or OOM kill) immediately after a sensitive process event is detected can erase the audit evidence. The WAL claims the event was persisted (returns the sequence number), but the data never reached stable storage. - -**Proof**: Grepping for `flush`, `sync_data`, and `sync_all` in `procmond/src/wal.rs` returns zero matches. - -**Remediation**: - -1. Add `state.file.flush().await.map_err(WalError::Io)?;` after `write_all` in both `write()` and `write_with_type()`. -2. Add `state.file.sync_data().await.map_err(WalError::Io)?;` after flush for crash durability. -3. Consider a configurable sync mode (sync-per-write vs. periodic sync) to balance durability against throughput. - ---- - -### CRITICAL-02: UTF-8 Byte Slicing Panic in CorrelationMetadata - -| Attribute | Value | -| ------------ | ----------------------------------------------------------- | -| **Severity** | Critical | -| **CWE** | CWE-135 (Incorrect Calculation of Multi-Byte String Length) | -| **File** | `daemoneye-eventbus/src/message.rs`, line 36 | - -**Description**: `CorrelationMetadata::new()` truncates oversized correlation IDs using byte indexing: - -```rust -let bounded_id = if correlation_id.len() > MAX_CORRELATION_ID_LENGTH { - correlation_id[..MAX_CORRELATION_ID_LENGTH].to_string() -} else { - correlation_id -}; -``` - -The `String::len()` method returns byte length, not character count. If `MAX_CORRELATION_ID_LENGTH` (256) falls on a multi-byte UTF-8 boundary (e.g., within a 2-, 3-, or 4-byte character), this will panic at runtime with `byte index N is not a char boundary`. - -**Attack Scenario**: An attacker sends a crafted correlation ID containing multi-byte characters (e.g., emoji or CJK characters) positioned so that byte offset 256 falls mid-character. This causes a panic in the eventbus, crashing the daemoneye-agent process. Note: the workspace lint `panic = "deny"` prevents explicit `panic!()` calls but does not prevent runtime panics from invalid byte slicing — those are a distinct class of runtime error. - -**Remediation**: - -1. Replace byte slicing with `correlation_id.char_indices()` to find the safe truncation point: - -```rust -let bounded_id = if correlation_id.len() > MAX_CORRELATION_ID_LENGTH { - let end = correlation_id - .char_indices() - .take_while(|(i, _)| *i < MAX_CORRELATION_ID_LENGTH) - .last() - .map_or(0, |(i, c)| i + c.len_utf8()); - correlation_id[..end].to_owned() -} else { - correlation_id -}; -``` - -2. Alternatively, use `correlation_id.floor_char_boundary(MAX_CORRELATION_ID_LENGTH)` when stabilized. - ---- - -### CRITICAL-03: Privilege Separation Violation -- CLI Has Write Access - -| Attribute | Value | -| ------------ | -------------------------------------------------------------------------- | -| **Severity** | Critical | -| **CWE** | CWE-269 (Improper Privilege Management), CWE-284 (Improper Access Control) | -| **File** | `daemoneye-cli/src/main.rs`, line 42 | - -**Description**: The architecture document explicitly states the CLI must have **read-only** database access. However, `daemoneye-cli/src/main.rs` calls `DatabaseManager::new(&database_path)` which invokes `Database::create()` (line 180 of `storage.rs`) and `initialize_schema()` -- both are write operations. - -```rust -// daemoneye-cli/src/main.rs:42 -let db_manager = storage::DatabaseManager::new(&database_path)?; -``` - -There is no `ReadOnlyDatabaseManager` type or read-only database accessor. The `DatabaseManager` struct exposes full read/write operations (`store_process`, `store_alert`, `store_rule`, etc.) to any caller that holds a reference. - -**Attack Scenario**: If the CLI binary is compromised or a bug is introduced, it could write to or corrupt the event store database. This violates the principle of least privilege and breaks the security boundary between components. - -**Remediation**: - -1. Create a `ReadOnlyDatabaseManager` that wraps `Database::open()` (not `create`) and only exposes read methods. -2. Change `daemoneye-cli` to use `DatabaseManager::open()` instead of `::new()`. -3. Enforce at the type level: the CLI should never have access to write methods. Consider using trait-based access control (e.g., `ReadableStorage` vs `WritableStorage` traits). - ---- - -### HIGH-01: Missing Workspace Lint Inheritance in Two Crates - -| Attribute | Value | -| ------------ | ------------------------------------------------------------ | -| **Severity** | High | -| **CWE** | CWE-710 (Improper Adherence to Coding Standards) | -| **Files** | `collector-core/Cargo.toml`, `daemoneye-eventbus/Cargo.toml` | - -**Description**: The workspace root defines security-critical lint rules including `unsafe_code = "forbid"`, `panic = "deny"`, `unwrap_used = "deny"`, and `await_holding_lock = "deny"`. However, `collector-core` and `daemoneye-eventbus` do not include `[lints] workspace = true` in their `Cargo.toml` files, meaning they do not inherit these security lints. - -The remaining 4 crates (`daemoneye-agent`, `daemoneye-cli`, `daemoneye-lib`, `procmond`) all properly inherit workspace lints. - -**Impact**: Code in these two crates can use `unsafe`, `.unwrap()`, `.panic!()`, and hold locks across await points without any compiler-level enforcement. Since `daemoneye-eventbus` handles the IPC transport layer and `collector-core` manages process collection, these are security-critical paths. - -**Remediation**: Add to both `collector-core/Cargo.toml` and `daemoneye-eventbus/Cargo.toml`: - -```toml -[lints] -workspace = true -``` - ---- - -### HIGH-02: Eventbus Transport Socket Created Without Restricted Permissions - -| Attribute | Value | -| ------------ | --------------------------------------------------------------- | -| **Severity** | High | -| **CWE** | CWE-732 (Incorrect Permission Assignment for Critical Resource) | -| **File** | `daemoneye-eventbus/src/transport.rs`, lines 166-213 | - -**Description**: `TransportServer::new()` creates a Unix domain socket via `ListenerOptions::new().name(name).create_tokio()` but never sets file permissions on the resulting socket. The default umask typically creates sockets with world-readable/writable permissions (e.g., 0o755 or 0o777). - -By contrast, the `daemoneye-lib/src/ipc/interprocess_transport.rs` correctly restricts socket permissions to `0o600` (owner-only) and its parent directory to `0o700`. The eventbus transport has no such hardening. - -**Attack Scenario**: A local unprivileged user on the same system can connect to the eventbus socket and inject or intercept process monitoring events, potentially injecting false positives to mask real threats or causing denial of service. - -**Remediation**: - -1. After creating the listener, set socket permissions to `0o600`: - -```rust -#[cfg(unix)] -{ - use std::os::unix::fs::PermissionsExt; - let perms = std::fs::Permissions::from_mode(0o600); - std::fs::set_permissions(&socket_path, perms)?; -} -``` - -2. Set the parent directory to `0o700` if it is newly created. - ---- - -### HIGH-03: Storage Layer Entirely Stubbed -- Detection Pipeline Operates on Phantom Data - -| Attribute | Value | -| ------------ | -------------------------------------------------------------- | -| **Severity** | High | -| **CWE** | CWE-754 (Improper Check for Unusual or Exceptional Conditions) | -| **File** | `daemoneye-lib/src/storage.rs`, lines 216-384 | - -**Description**: Every `DatabaseManager` method that should persist or retrieve data is stubbed: - -- All `store_*` methods open a write transaction but do nothing and return `Ok(())`. -- All `get_*` methods open a read transaction but return `Ok(None)` or `Ok(Vec::new())`. -- `initialize_schema()` begins a transaction but creates no tables. -- `cleanup_old_data()` always returns `Ok(0)`. - -This means the entire detection pipeline -- from rule storage to alert delivery -- operates on phantom data. Alerts are generated but never persisted. Process records are "stored" but immediately lost. - -**Impact**: Any downstream component that relies on stored data (the detection engine, the CLI, audit trail verification) is receiving empty results, creating a false sense of security. The system appears to be monitoring but is not retaining evidence. - -**Remediation**: Implement the storage operations for Task 8 as documented in the code TODOs. Until then, add explicit warnings or error returns to prevent callers from silently succeeding without actual persistence. - ---- - -### HIGH-04: Unchecked `as` Casts in collector-core - -| Attribute | Value | -| ------------ | ---------------------------------------------------------------------------------------------------------------------------- | -| **Severity** | High | -| **CWE** | CWE-681 (Incorrect Conversion between Numeric Types), CWE-190 (Integer Overflow) | -| **Files** | `collector-core/src/trigger.rs`, `collector-core/src/performance.rs`, `collector-core/src/transport.rs` (multiple locations) | - -**Description**: The `collector-core` crate contains numerous bare `as` casts without `#[allow(clippy::as_conversions)]` annotations or safety comments. Because `collector-core` does not inherit workspace lints (see HIGH-01), the `as_conversions = "warn"` lint is not enforced. Examples: - -- `trigger.rs:1486`: `(self.max_queue_size as f32 * self.backpressure_threshold) as usize` -- if the product exceeds `usize::MAX`, this silently truncates. -- `trigger.rs:1776`: `elapsed.as_micros() as u64` -- `as_micros()` returns `u128`, truncation to `u64` is lossy for durations > 585,000 years (unlikely but contract-violating). -- `trigger.rs:2440`: `i as u32` -- loop index cast without bounds check. -- `performance.rs:490`: `(0.95 * sorted_samples.len() as f64).ceil() as usize` -- floating point to usize cast with potential NaN/infinity issues. - -**Remediation**: - -1. First, fix HIGH-01 to enable lint inheritance for `collector-core`. -2. Replace bare `as` casts with `u32::try_from(i).unwrap_or(0)`, `.min(u64::MAX.into())`, or `.saturating_*` methods. -3. Add `#[allow(clippy::as_conversions)]` with safety comments for intentional casts. - ---- - -### HIGH-05: Silent Event Type Fallback to Start - -| Attribute | Value | -| ------------ | ---------------------------------------------------- | -| **Severity** | High | -| **CWE** | CWE-393 (Return of Wrong Status Code) | -| **File** | `procmond/src/event_bus_connector.rs`, lines 175-186 | - -**Description**: The `from_type_string()` method defaults unknown event types to `Start`: - -```rust -fn from_type_string(s: &str) -> Self { - match s { - "start" => Self::Start, - "stop" => Self::Stop, - "modify" => Self::Modify, - _ => { - warn!(event_type = s, "Unknown event type, defaulting to Start"); - Self::Start - } - } -} -``` - -A `Stop` event that is corrupted or arrives with an unknown type string will be silently reclassified as a `Start` event. This can cause the detection engine to miss process termination events, maintain stale process records, and generate false alerts about processes that no longer exist. - -**Remediation**: - -1. Return a `Result` or a dedicated `Unknown` variant instead of silently defaulting. -2. If backward compatibility requires a default, use a distinct `Unknown` variant that the detection engine can filter or flag separately. - ---- - -### MEDIUM-01: EventBus Transport Authentication Is Optional and Unenforced - -| Attribute | Value | -| ------------ | ----------------------------------------------------------- | -| **Severity** | Medium | -| **CWE** | CWE-306 (Missing Authentication for Critical Function) | -| **File** | `daemoneye-eventbus/src/transport.rs`, lines 62-63, 128-129 | - -**Description**: `SocketConfig` has an `auth_token: Option` field and `ClientConfig` has a matching `auth_token: Option`, but authentication is only used during health checks (lines 758-767). The `accept()`, `send()`, and `receive()` methods do not validate any authentication token. A client can connect, send messages, and receive data without presenting any credentials. - -**Remediation**: Implement mandatory token-based authentication as part of the connection handshake. Reject unauthenticated connections in `accept()`. - ---- - -### MEDIUM-02: Audit Ledger Hash Chain Uses Timestamp Seconds -- Collision Window - -| Attribute | Value | -| ------------ | ------------------------------------------ | -| **Severity** | Medium | -| **CWE** | CWE-328 (Use of Weak Hash) | -| **File** | `daemoneye-lib/src/crypto.rs`, lines 72-83 | - -**Description**: The audit entry hash is computed over a formatted string that includes `timestamp.timestamp()` (Unix seconds). Two audit entries created within the same second with identical actor, action, and payload will produce identical entry hashes, potentially allowing one to be substituted for the other without detection. - -```rust -let entry_data = format!( - "{}:{}:{}:{}:{}:{}", - sequence, timestamp.timestamp(), actor, action, - payload_hash, previous_hash.as_deref().unwrap_or("") -); -``` - -**Remediation**: Use `timestamp.timestamp_nanos_opt()` or `timestamp.to_rfc3339()` for sub-second precision. Alternatively, include a random nonce in the hash computation. - ---- - -### MEDIUM-03: Merkle Tree Inclusion Proof Is Unimplemented - -| Attribute | Value | -| ------------ | -------------------------------------------------------- | -| **Severity** | Medium | -| **CWE** | CWE-345 (Insufficient Verification of Data Authenticity) | -| **File** | `daemoneye-lib/src/crypto.rs`, lines 127-131 | - -**Description**: `AuditLedger::generate_inclusion_proof()` is documented as providing "Merkle tree-based verification with logarithmic proof sizes" but returns an empty `Vec`: - -```rust -pub const fn generate_inclusion_proof(_index: usize) -> Vec { - vec![] -} -``` - -The `rs_merkle` crate is listed as a dependency but is never used. The audit ledger is a simple hash chain, not a Merkle tree. This means tamper detection requires verifying the entire chain (O(n)) rather than an inclusion proof (O(log n)). - -**Remediation**: Either implement the Merkle tree using `rs_merkle` as designed, or update documentation to accurately reflect the current hash chain implementation. - ---- - -### MEDIUM-04: Configuration Validation Is Minimal - -| Attribute | Value | -| ------------ | -------------------------------------------- | -| **Severity** | Medium | -| **CWE** | CWE-20 (Improper Input Validation) | -| **File** | `daemoneye-lib/src/config.rs`, lines 663-683 | - -**Description**: `validate_config()` only checks three fields: - -- `scan_interval_ms != 0` -- `batch_size != 0` -- `retention_days != 0` - -It does not validate: - -- `database.path` for path traversal or dangerous characters -- `logging.level` for valid log level values -- `alerting.sinks[].sink_type` for known sink types -- `broker.socket_path` for path injection -- `broker.max_connections` for unreasonably high values (DoS) -- `database.max_size_mb` for unreasonably high values -- File permissions on config file itself - -**Remediation**: Add comprehensive validation for all security-relevant configuration fields. Validate path strings, clamp numeric ranges, and reject unknown sink types. - ---- - -### MEDIUM-05: Command-Line Sanitizer Does Not Handle `=` Syntax - -| Attribute | Value | -| ------------ | ------------------------------------------- | -| **Severity** | Medium | -| **CWE** | CWE-200 (Exposure of Sensitive Information) | -| **File** | `procmond/src/security.rs`, lines 371-392 | - -**Description**: `sanitize_command_line()` splits on whitespace and checks if the previous token was a sensitive flag. However, it does not handle the common `--flag=value` syntax: - -```text -app --password=secret123 --verbose -``` - -This passes through unsanitized because `--password=secret123` is a single token that does not exactly match `--password`. - -**Remediation**: Add parsing for `=`-separated flag-value pairs: - -```rust -} else if let Some((flag, _value)) = token.split_once('=') { - if SENSITIVE_FLAGS.iter().any(|f| flag.eq_ignore_ascii_case(f)) { - result.push(&format!("{flag}={REDACTED}")); - } else { - result.push(token); - } -} -``` - ---- - -### MEDIUM-06: Exponential Backoff Reconnect Uses Unchecked `as` Casts - -| Attribute | Value | -| ------------ | ---------------------------------------------------- | -| **Severity** | Medium | -| **CWE** | CWE-681 (Incorrect Conversion between Numeric Types) | -| **File** | `daemoneye-eventbus/src/transport.rs`, lines 545-554 | - -**Description**: The reconnection backoff calculation uses chained `as` casts through floating point: - -```rust -let delay = Duration::from_millis( - (self.config.initial_reconnect_delay.as_millis() as f64 - * self.config.backoff_multiplier - .powi(self.reconnect_attempts as i32)) as u64, -); -``` - -- `as_millis()` returns `u128`, cast to `f64` loses precision for large values. -- `reconnect_attempts as i32` overflows if attempts > `i32::MAX`. -- The final `as u64` can produce `u64::MAX` from `f64::INFINITY` or `0` from `NaN`. - -**Remediation**: Use `u64::try_from()` with bounds checking, or `u128::min()` before the cast. Cap `reconnect_attempts` to prevent overflow of `powi`. - ---- - -### LOW-01: GitHub Actions Not Pinned to SHA in Secondary Workflows - -| Attribute | Value | -| ------------ | -------------------------------------------------------------------------------------------------- | -| **Severity** | Low | -| **CWE** | CWE-829 (Inclusion of Functionality from Untrusted Control Sphere) | -| **Files** | `.github/workflows/security.yml`, `audit.yml`, `docs.yml`, `codeql.yml`, `copilot-setup-steps.yml` | - -**Description**: The primary `ci.yml`, `release.yml`, and `benchmarks.yml` workflows correctly pin actions to SHA (e.g., `actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd`). However, the secondary workflows use mutable tags (e.g., `actions/checkout@v6`, `jdx/mise-action@v3`). - -**Remediation**: Pin all action references to full SHA hashes across all workflows. Use Dependabot or Renovate to automatically update the SHAs. - ---- - -### LOW-02: Default Encryption Disabled for Database - -| Attribute | Value | -| ------------ | ---------------------------------------------- | -| **Severity** | Low | -| **CWE** | CWE-311 (Missing Encryption of Sensitive Data) | -| **File** | `daemoneye-lib/src/config.rs`, line 260 | - -**Description**: `DatabaseConfig::default()` sets `encryption_enabled: false`. Process metadata, executable hashes, and command lines stored in the database are sensitive. While redb does not natively support encryption, the config field suggests encryption was planned. - -**Remediation**: Document the encryption roadmap. If encryption is not yet supported, either remove the field to avoid false expectations or implement it using an encryption layer over the redb file (e.g., filesystem-level encryption guidance in deployment docs). - ---- - -### LOW-03: Eventbus Socket Path Uses `/tmp` by Default - -| Attribute | Value | -| ------------ | --------------------------------------------------------------------------- | -| **Severity** | Low | -| **CWE** | CWE-379 (Creation of Temporary File in Directory with Insecure Permissions) | -| **File** | `daemoneye-eventbus/src/transport.rs`, line 77 | - -**Description**: `SocketConfig::new()` constructs the default socket path as `/tmp/daemoneye-{instance_id}.sock`. The `/tmp` directory is world-writable, and without proper socket permissions (see HIGH-02), any local user can interact with the socket. - -**Remediation**: Use a dedicated runtime directory (e.g., `/var/run/daemoneye/`) with restricted permissions, consistent with how `daemoneye-lib` handles IPC paths. - ---- - -### LOW-04: `AuditLedger` Is Not Persistent - -| Attribute | Value | -| ------------ | ----------------------------------------------- | -| **Severity** | Low | -| **CWE** | CWE-404 (Improper Resource Shutdown or Release) | -| **File** | `daemoneye-lib/src/crypto.rs`, lines 101-104 | - -**Description**: `AuditLedger` stores entries in an in-memory `Vec`. There is no serialization, persistence, or recovery mechanism. If the process restarts, the entire audit ledger is lost. - -**Remediation**: Implement persistence to the WAL or database. The `verify_integrity()` method is useful but only works on the in-memory state. - ---- - -## Positive Security Observations - -01. **`unsafe_code = "forbid"`** at workspace level with `overflow-checks = true` in all profiles (dev, release, dist). -02. **AST-based SQL validation** using `sqlparser` with banned function lists, statement type restrictions (SELECT only), and structural limits (max joins, max columns). -03. **CRC32 integrity validation** on IPC frames with size limits and timeout enforcement. -04. **WAL file permissions** are correctly restricted to `0o600` with TOCTOU mitigation via `OpenOptions::mode()`. -05. **Comprehensive clippy lint configuration** including `unwrap_used = "deny"`, `panic = "deny"`, `await_holding_lock = "deny"`. -06. **`cargo deny`** configuration bans `openssl` in favor of `rustls`, denies yanked crates, and restricts to known registries. -07. **Pinned action SHAs** in primary CI workflow (`ci.yml`, `release.yml`, `benchmarks.yml`). -08. **Data sanitization** for command lines, environment variables, and file paths with redaction of sensitive patterns. -09. **No `unsafe` code** across the entire workspace (enforced by `forbid`). -10. **Socket permissions hardened** on the IPC transport layer in `daemoneye-lib` (0o600 socket, 0o700 directory). - ---- - -## Summary Table - -| ID | Severity | CWE | Component | Finding | -| ----------- | -------- | ----------- | ---------------------------------- | ------------------------------------------------------------------ | -| CRITICAL-01 | Critical | CWE-311/755 | procmond/wal.rs | WAL missing fsync -- audit trail data loss on crash | -| CRITICAL-02 | Critical | CWE-135 | daemoneye-eventbus/message.rs | UTF-8 byte slicing panic in CorrelationMetadata | -| CRITICAL-03 | Critical | CWE-269/284 | daemoneye-cli/main.rs | CLI has write access to database (architecture requires read-only) | -| HIGH-01 | High | CWE-710 | collector-core, daemoneye-eventbus | Missing `[lints] workspace = true` -- security lints unenforced | -| HIGH-02 | High | CWE-732 | daemoneye-eventbus/transport.rs | Eventbus socket created without restricted permissions | -| HIGH-03 | High | CWE-754 | daemoneye-lib/storage.rs | All storage methods stubbed -- detection pipeline on phantom data | -| HIGH-04 | High | CWE-681/190 | collector-core (multiple files) | Unchecked `as` casts in security-critical arithmetic | -| HIGH-05 | High | CWE-393 | procmond/event_bus_connector.rs | Unknown event types silently default to Start | -| MEDIUM-01 | Medium | CWE-306 | daemoneye-eventbus/transport.rs | Authentication optional and unenforced on IPC connections | -| MEDIUM-02 | Medium | CWE-328 | daemoneye-lib/crypto.rs | Audit hash uses second-precision timestamps -- collision window | -| MEDIUM-03 | Medium | CWE-345 | daemoneye-lib/crypto.rs | Merkle tree inclusion proof unimplemented (returns empty vec) | -| MEDIUM-04 | Medium | CWE-20 | daemoneye-lib/config.rs | Configuration validation is minimal -- missing path/range checks | -| MEDIUM-05 | Medium | CWE-200 | procmond/security.rs | Command sanitizer misses `--flag=value` syntax | -| MEDIUM-06 | Medium | CWE-681 | daemoneye-eventbus/transport.rs | Unchecked `as` casts in exponential backoff calculation | -| LOW-01 | Low | CWE-829 | .github/workflows/ | GitHub Actions not pinned to SHA in 5 secondary workflows | -| LOW-02 | Low | CWE-311 | daemoneye-lib/config.rs | Database encryption disabled by default | -| LOW-03 | Low | CWE-379 | daemoneye-eventbus/transport.rs | Eventbus socket defaults to `/tmp` | -| LOW-04 | Low | CWE-404 | daemoneye-lib/crypto.rs | AuditLedger is memory-only with no persistence | - ---- - -## Recommended Prioritization - -**Immediate (before next release)**: - -1. CRITICAL-02 -- Fix UTF-8 panic (single-line fix, prevents DoS) -2. HIGH-01 -- Add `[lints] workspace = true` to 2 crates (trivial fix, high impact) -3. HIGH-02 -- Set socket permissions on eventbus transport - -**Short-term (next sprint)**: 4. CRITICAL-01 -- Add fsync to WAL writes 5. CRITICAL-03 -- Implement read-only database accessor for CLI 6. HIGH-05 -- Replace silent event type fallback with error/Unknown variant 7. MEDIUM-05 -- Fix `--flag=value` sanitization gap - -**Medium-term (next milestone)**: 8. HIGH-03 -- Implement storage layer (Task 8) 9. HIGH-04 -- Audit and fix all bare `as` casts in collector-core 10. MEDIUM-01 through MEDIUM-06 -- Remaining medium findings 11. LOW-01 through LOW-04 -- Low severity items diff --git a/collector-core/Cargo.toml b/collector-core/Cargo.toml index 88f03997..b4b68848 100644 --- a/collector-core/Cargo.toml +++ b/collector-core/Cargo.toml @@ -29,6 +29,10 @@ postcard = { workspace = true } # Bitflags for capabilities bitflags = { workspace = true } +# Capability-based filesystem access (TOCTOU-safe opens) +cap-fs-ext = { workspace = true } +cap-std = { workspace = true } + # High-performance concurrent data structures crossbeam = { workspace = true } diff --git a/collector-core/benches/collector_benchmarks.rs b/collector-core/benches/collector_benchmarks.rs index 86649a74..2ac83a21 100644 --- a/collector-core/benches/collector_benchmarks.rs +++ b/collector-core/benches/collector_benchmarks.rs @@ -127,6 +127,7 @@ impl EventSource for BenchmarkEventSource { cpu_usage: Some(1.5), memory_usage: Some(1024 * 1024), executable_hash: Some("bench_hash".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -481,6 +482,7 @@ fn bench_performance_monitoring_overhead(c: &mut Criterion) { cpu_usage: Some(1.5), memory_usage: Some(1024 * 1024), executable_hash: Some("bench_hash".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, diff --git a/collector-core/src/binary_hasher.rs b/collector-core/src/binary_hasher.rs new file mode 100644 index 00000000..1a2d0764 --- /dev/null +++ b/collector-core/src/binary_hasher.rs @@ -0,0 +1,1230 @@ +//! `BinaryHasherCollector` — the first [`TriggerableCollector`] implementation. +//! +//! Consumes [`TriggerRequest`]s with `analysis_type = AnalysisType::BinaryHash`, +//! authorizes the target path against a mandatory `allowed_roots` list, and +//! delegates hashing to [`daemoneye_lib::integrity::MultiAlgorithmHasher`]. +//! Returns an [`AnalysisResult`] containing a JSON-encoded `HashResult`. +//! +//! # Security model +//! +//! `TriggerRequest.target_path` is **untrusted input**: detection rules can +//! be influenced by attacker-controlled process paths, and rule authors may +//! misuse the trigger mechanism even without malicious intent. The collector +//! defends in depth: +//! +//! 1. **Mandatory allow-list with deny-on-empty**: +//! [`BinaryHasherConfig::allowed_roots`] must be non-empty. Requests for +//! paths outside every root return [`crate::triggerable::TriggerErrorKind::PathNotAllowed`]. +//! [`BinaryHasherConfig::with_platform_defaults`] provides secure defaults +//! for Linux, macOS, and Windows. +//! +//! 2. **Path length cap**: requests with `target_path.len() > 4096` are +//! rejected with [`crate::triggerable::TriggerErrorKind::InvalidRequest`] before any I/O. +//! +//! 3. **Parent-traversal rejection**: paths containing `..` components are +//! rejected before canonicalization to prevent path-traversal primitives. +//! +//! 4. **Symlink rejection** (default): paths that resolve through a symbolic +//! link, Windows junction, or reparse point return +//! [`crate::triggerable::TriggerErrorKind::Unavailable`]. Operators can opt in to symlink +//! following via [`BinaryHasherConfig::with_follow_symlinks`], but the +//! resolved target must still pass the `allowed_roots` check. +//! +//! 5. **Canonicalization + prefix match**: the requested path is +//! canonicalized and then verified to be under one of the configured +//! roots. On Windows, the `dunce` crate's UNC normalization would be +//! used — since we do not yet depend on it, Windows support is a +//! documented follow-up. +//! +//! 6. **Wire-error sanitization**: internal errors carry rich context +//! (paths, reasons) for local `tracing::warn!` logs. At the trait +//! boundary they map through [`TriggerHandleError::kind`] to the closed +//! [`crate::triggerable::TriggerErrorKind`] enum. +//! [`crate::triggerable::TriggerErrorKind::Unavailable`] deliberately merges "permission +//! denied" and "not found" to prevent file-existence oracles. +//! +//! 7. **Critical priority does NOT bypass authorization**: priority +//! affects queue ordering only. `allowed_roots`, symlink policy, and +//! resource budgets apply uniformly. +//! +//! # TOCTOU defense (cap-std) +//! +//! As of P1 Phase 3, path authorization uses cap-std `Dir` handles +//! opened at construction time (before privilege drop). Each allowed +//! root is a `Dir` handle; requests are opened relative to the root's +//! fd via `Dir::open()`, which the kernel resolves atomically against +//! the handle's subtree. This eliminates the `canonicalize()` → +//! `File::open()` TOCTOU gap. +//! +//! On macOS, an additional `(st_dev, st_ino)` fingerprint is recorded +//! at `Dir::open_ambient_dir` time and verified before each open. This +//! detects bind-mount / volume swaps (not atomic, but raises attacker +//! cost above zero). +//! +//! # Known gaps (follow-up work) +//! +//! - **Windows junction / reparse-point rejection**: requires calling the +//! Win32 `GetFileInformationByHandleEx` API via the `windows` crate; the +//! current stdlib-only path relies on `symlink_metadata().is_symlink()` +//! which does not cover all reparse-point types. + +use crate::analysis_chain::AnalysisResult; +use crate::event::{AnalysisType, TriggerRequest}; +use crate::triggerable::{TriggerHandleError, TriggerableCollector}; +use cap_std::fs::Dir; +use daemoneye_lib::integrity::{HashError, MultiAlgorithmHasher, auth::AuthError}; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; +use tracing::{debug, info, warn}; + +/// Hard cap on `TriggerRequest.target_path` length in bytes. Paths longer +/// than this are rejected with [`crate::triggerable::TriggerErrorKind::InvalidRequest`] before +/// any I/O. +pub const MAX_TARGET_PATH_LEN: usize = 4096; + +// ───────────────────────────────────────────────────────────────────────────── +// AllowedRoot — cap-std Dir handle + metadata +// ───────────────────────────────────────────────────────────────────────────── + +/// An allowed root directory opened via cap-std. +/// +/// Holds the display path (for logging), the kernel-level `Dir` handle +/// (for TOCTOU-safe opens), and on macOS a `(st_dev, st_ino)` fingerprint +/// recorded at open time to detect bind-mount / volume swaps. +/// +/// **Lifecycle invariant**: `AllowedRoot::open` MUST be called before +/// procmond drops privileges. The `Dir` handle persists across privilege +/// drops and confines all subsequent opens to the root's subtree at the +/// kernel level. +pub struct AllowedRoot { + /// Human-readable (canonical) path for logging. + display: String, + /// Original path before canonicalization (for `strip_prefix` on + /// macOS where `/var` → `/private/var`). + original: String, + /// Capability handle opened via `Dir::open_ambient_dir`. + handle: Dir, + /// On macOS, `(st_dev, st_ino)` recorded at open time. + #[cfg(target_os = "macos")] + fingerprint: (u64, u64), +} + +impl std::fmt::Debug for AllowedRoot { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("AllowedRoot") + .field("display", &self.display) + .finish_non_exhaustive() + } +} + +impl AllowedRoot { + /// Open a root directory via `Dir::open_ambient_dir`. + /// + /// On macOS, also records `(st_dev, st_ino)` for fingerprint + /// verification. + /// + /// # Errors + /// + /// Returns `std::io::Error` if the directory does not exist or + /// cannot be opened. + pub fn open(root: &Path) -> Result { + let original = root.display().to_string(); + // Canonicalize so strip_prefix works on macOS (/var → /private/var). + let canonical_root = std::fs::canonicalize(root)?; + let handle = Dir::open_ambient_dir(&canonical_root, cap_std::ambient_authority())?; + + #[cfg(target_os = "macos")] + let fingerprint = { + use std::os::unix::fs::MetadataExt; + let meta = std::fs::metadata(&canonical_root)?; + (meta.dev(), meta.ino()) + }; + + Ok(Self { + display: canonical_root.display().to_string(), + original, + handle, + #[cfg(target_os = "macos")] + fingerprint, + }) + } + + /// Borrow the cap-std `Dir` handle. + #[must_use] + pub const fn handle(&self) -> &Dir { + &self.handle + } + + /// Display path for logging. + #[must_use] + pub fn display_path(&self) -> &str { + &self.display + } + + /// On macOS, verify the fingerprint still matches. + #[cfg(target_os = "macos")] + pub fn verify_fingerprint(&self) -> Result<(), AuthError> { + use std::os::unix::fs::MetadataExt; + let current = + std::fs::metadata(Path::new(&self.display)).map_err(|source| AuthError::Io { + path: PathBuf::from(&self.display), + source, + })?; + let current_fp = (current.dev(), current.ino()); + if current_fp != self.fingerprint { + return Err(AuthError::RootMountChanged { + root: self.display.clone(), + }); + } + Ok(()) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// authorize_confined_path — cap-std gated authorization +// ───────────────────────────────────────────────────────────────────────────── + +/// The well-known cap-std error message for path escapes. +/// +/// Regression-tested to catch upstream message changes on cap-std +/// upgrades. +pub const CAP_STD_ESCAPE_MESSAGE: &str = "a path led outside of the filesystem"; + +/// Verify fingerprints for all allowed roots on macOS. +/// +/// This MUST be called once before entering any per-file hash stream to +/// detect bind-mount / volume swaps for all roots up front, rather than +/// checking inside the per-file loop. Callers (e.g. +/// `BinaryHasherCollector::authorize`) should invoke this before +/// calling [`authorize_confined_path`]. +/// +/// On non-macOS platforms this is a no-op that always returns `Ok(())`. +/// +/// # Errors +/// +/// Returns `AuthError::RootMountChanged` if any root's `(st_dev, st_ino)` +/// no longer matches the fingerprint recorded at open time, or +/// `AuthError::Io` if the root metadata cannot be read. +#[allow( + clippy::missing_const_for_fn, + reason = "not const on macOS where verify_fingerprint() is called" +)] +pub fn verify_all_fingerprints(roots: &[AllowedRoot]) -> Result<(), AuthError> { + #[cfg(target_os = "macos")] + for root in roots { + root.verify_fingerprint()?; + } + // Suppress unused-variable warning on non-macOS. + #[cfg(not(target_os = "macos"))] + let _ = roots; + Ok(()) +} + +/// Authorize and open a target path via cap-std `Dir` handles. +/// +/// Tries each allowed root in order. For each root: +/// 1. Strip the root prefix from the target to get a relative path. +/// 2. Open the relative path via `root.handle().open()`. +/// 3. Fetch metadata from the opened file handle (fstat — no second path stat). +/// +/// Returns both the opened file and its metadata so callers can perform +/// file-type and size checks without issuing a second path-based stat(2). +/// +/// If the path escapes (cap-std returns the escape error), this is +/// mapped to [`AuthError::CapStdEscape`]. If no root matches, returns +/// [`AuthError::PathNotAllowed`]. +/// +/// **Fingerprint pre-check**: On macOS, callers MUST call +/// [`verify_all_fingerprints`] once before entering the per-file stream. +/// `authorize_confined_path` does NOT re-verify fingerprints to avoid +/// redundant stat calls on every file in a batch. +/// +/// # Errors +/// +/// Returns [`AuthError`] on escape, I/O errors, or if the path is not +/// under any allowed root. +pub fn authorize_confined_path( + target: &Path, + roots: &[AllowedRoot], + follow_symlinks: bool, +) -> Result<(cap_std::fs::File, cap_std::fs::Metadata, PathBuf), AuthError> { + use daemoneye_lib::integrity::auth; + + // Pre-flight checks using shared predicates. + auth::check_path_length(target)?; + auth::check_no_traversal(target)?; + + for root in roots { + // WHY two strip_prefix attempts? + // + // On macOS, `/var` is a symlink to `/private/var`. `AllowedRoot::open` + // calls `std::fs::canonicalize` on the configured root path, so the + // `Dir` handle is always opened against the real `/private/var/...` + // path and `display` (the `canonical_root` here) will always start + // with `/private/var`. However, process enumeration via `sysinfo` (or + // other OS APIs) may return the *un-resolved* `/var/...` form for the + // executable path, depending on how the kernel surfaces it. + // + // To bridge that gap we keep the pre-canonicalization path in + // `AllowedRoot::original` and try stripping both prefixes. If either + // matches we obtain a valid relative path and proceed. + // + // SECURITY: This dual-try is NOT a confinement bypass. `strip_prefix` + // only computes the *relative* path that is handed to cap-std's + // `Dir::open` / `Dir::open_with`. The confinement guarantee is + // enforced entirely by the kernel: the `Dir` fd was opened at + // `AllowedRoot::open` time (canonicalized, before any privilege drop), + // and all subsequent `open` calls are resolved *relative to that fd*. + // The kernel will refuse traversal outside the fd's subtree regardless + // of which string prefix we stripped. cap-std additionally detects any + // escape attempt and returns [`CAP_STD_ESCAPE_MESSAGE`], which we map + // to [`AuthError::CapStdEscape`] below. + // + // The `original` field on `AllowedRoot` exists specifically for this + // macOS `/var` ↔ `/private/var` compatibility case. + let canonical_root = Path::new(root.display_path()); + let original_root = Path::new(&root.original); + let relative = if let Ok(rel) = target.strip_prefix(canonical_root) { + rel + } else if let Ok(rel) = target.strip_prefix(original_root) { + rel + } else { + continue; + }; + + // Open via cap-std. This is TOCTOU-safe: the kernel resolves + // the path relative to the Dir handle's fd. + let open_result = if follow_symlinks { + // Use cap-fs-ext to explicitly follow symlinks on the + // final component. + use cap_fs_ext::{FollowSymlinks, OpenOptionsFollowExt}; + let mut opts = cap_std::fs::OpenOptions::new(); + opts.read(true); + opts.follow(FollowSymlinks::Yes); + root.handle().open_with(relative, &opts) + } else { + // Check symlink_metadata first to reject symlinks before + // opening. + let meta_result = root.handle().symlink_metadata(relative); + match meta_result { + Ok(ref meta) if meta.is_symlink() => { + return Err(AuthError::SymlinkRejected { + path: target.to_path_buf(), + }); + } + Err(ref err) => { + return Err(cap_std_err_to_auth(err, target)); + } + Ok(_) => root.handle().open(relative), + } + }; + + match open_result { + Ok(file) => { + let meta = file.metadata().map_err(|source| AuthError::Io { + path: target.to_path_buf(), + source, + })?; + // Compose the audit-record path from the cap-std root's + // canonical display path joined with the relative path we + // just authorized. This avoids a second path-based stat + // (canonicalize) after the fd-based authorization, closing + // the cosmetic race where `recorded_path` could mismatch + // the inode actually hashed if the path was swapped + // between open and canonicalize. + let resolved = canonical_root.join(relative); + return Ok((file, meta, resolved)); + } + Err(err) => { + // Delegate to the helper so escape detection walks the + // full source chain rather than only matching the leaf + // error's `to_string()`. + return Err(cap_std_err_to_auth(&err, target)); + } + } + } + + Err(AuthError::PathNotAllowed { + path: target.to_path_buf(), + }) +} + +/// Map a cap-std I/O error to [`AuthError`]. +/// +/// Walks the full [`std::error::Error::source`] chain so an escape +/// detection stays correct even if cap-std upgrades start wrapping the +/// leaf message inside a higher-level error. The +/// `cap_std_escape_error_contains_expected_message` regression test +/// (in this file's `tests` module) pins the exact leaf message format +/// so we get a loud signal when the upstream text drifts. +fn cap_std_err_to_auth(err: &std::io::Error, target: &Path) -> AuthError { + if is_cap_std_escape(err) { + return AuthError::CapStdEscape { + message: err.to_string(), + }; + } + AuthError::Io { + path: target.to_path_buf(), + source: std::io::Error::new(err.kind(), err.to_string()), + } +} + +/// Returns `true` if `err` or any wrapped cause contains +/// [`CAP_STD_ESCAPE_MESSAGE`]. +fn is_cap_std_escape(err: &(dyn std::error::Error + 'static)) -> bool { + if err.to_string().contains(CAP_STD_ESCAPE_MESSAGE) { + return true; + } + let mut source = err.source(); + while let Some(inner) = source { + if inner.to_string().contains(CAP_STD_ESCAPE_MESSAGE) { + return true; + } + source = inner.source(); + } + false +} + +// ───────────────────────────────────────────────────────────────────────────── +// BinaryHasherConfig +// ───────────────────────────────────────────────────────────────────────────── + +/// Configuration for [`BinaryHasherCollector`]. +#[derive(Debug, Clone)] +pub struct BinaryHasherConfig { + /// Mandatory allow-list of root directories (as paths). + /// + /// These are opened as cap-std `Dir` handles at construction time. + /// **Empty list denies all requests.** Use + /// [`BinaryHasherConfig::with_platform_defaults`] for secure defaults. + pub allowed_roots: Vec, + /// When `false` (default), any path that resolves through a symbolic + /// link or junction is rejected with `Unavailable`. When `true`, the + /// resolved real path must still pass the `allowed_roots` check. + pub follow_symlinks: bool, + /// Maximum accepted file size, enforced before any read. + pub max_file_size: u64, + /// Per-file hashing deadline propagated to the engine. + pub timeout_per_file: Duration, + /// When `false` (default), startup aborts if any configured root + /// fails to open. When `true`, roots that fail to open are skipped + /// with a warning. + pub allow_partial_roots: bool, +} + +impl Default for BinaryHasherConfig { + fn default() -> Self { + Self { + allowed_roots: Vec::new(), + follow_symlinks: false, + max_file_size: 512 * 1024 * 1024, + timeout_per_file: Duration::from_secs(10), + allow_partial_roots: false, + } + } +} + +impl BinaryHasherConfig { + /// Builder: populate `allowed_roots` with platform-specific secure + /// defaults. + /// + /// On Unix: `/usr/bin`, `/usr/sbin`, `/usr/local/bin`, `/bin`, `/sbin`, + /// `/opt`. On macOS, also `/Applications` and `/System/Applications`. + /// On Windows: resolves `%SystemRoot%\\System32` and `%ProgramFiles%` + /// from the environment if available. + /// + /// Roots that do not exist on the current system are still added — + /// they won't match any request but keep the configuration portable + /// across CI matrices. + #[must_use] + pub fn with_platform_defaults(mut self) -> Self { + #[cfg(unix)] + self.allowed_roots.extend_from_slice(&[ + PathBuf::from("/usr/bin"), + PathBuf::from("/usr/sbin"), + PathBuf::from("/usr/local/bin"), + PathBuf::from("/bin"), + PathBuf::from("/sbin"), + PathBuf::from("/opt"), + ]); + #[cfg(target_os = "macos")] + self.allowed_roots.extend_from_slice(&[ + PathBuf::from("/Applications"), + PathBuf::from("/System/Applications"), + ]); + #[cfg(windows)] + { + if let Some(system_root) = std::env::var_os("SystemRoot") { + self.allowed_roots + .push(PathBuf::from(system_root).join("System32")); + } + if let Some(program_files) = std::env::var_os("ProgramFiles") { + self.allowed_roots.push(PathBuf::from(program_files)); + } + } + self + } + + /// Builder: add a single allowed root. The root path is stored as-is; + /// request canonicalization is performed at authorization time. + #[must_use] + pub fn with_allowed_root(mut self, root: impl Into) -> Self { + self.allowed_roots.push(root.into()); + self + } + + /// Builder: toggle symlink-following. **Leave at `false` for forensic + /// use unless operators opt in.** + #[must_use] + pub const fn with_follow_symlinks(mut self, follow: bool) -> Self { + self.follow_symlinks = follow; + self + } + + /// Builder: override the maximum accepted file size. + #[must_use] + pub const fn with_max_file_size(mut self, bytes: u64) -> Self { + self.max_file_size = bytes; + self + } + + /// Builder: override the per-file timeout. + #[must_use] + pub const fn with_timeout(mut self, timeout: Duration) -> Self { + self.timeout_per_file = timeout; + self + } + + /// Validate the configuration. + /// + /// # Errors + /// + /// Returns [`TriggerHandleError::Internal`] if any field is invalid. + /// Most notably, an empty `allowed_roots` list **must** fail validation + /// so an operator cannot accidentally deploy a deny-all-or-allow-all + /// misconfiguration. + pub fn validate(&self) -> Result<(), TriggerHandleError> { + if self.allowed_roots.is_empty() { + return Err(TriggerHandleError::Internal( + "allowed_roots must be non-empty - use with_platform_defaults() \ + or add explicit roots" + .to_owned(), + )); + } + if self.max_file_size == 0 { + return Err(TriggerHandleError::Internal( + "max_file_size must be greater than zero".to_owned(), + )); + } + if self.timeout_per_file.is_zero() { + return Err(TriggerHandleError::Internal( + "timeout_per_file must be greater than zero".to_owned(), + )); + } + Ok(()) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// BinaryHasherCollector +// ───────────────────────────────────────────────────────────────────────────── + +/// Reactive collector that handles [`AnalysisType::BinaryHash`] requests. +/// +/// Holds an `Arc` shared with any other caller of the +/// same engine so the configured concurrency cap and cache are honored +/// across the whole process. +/// +/// The `opened_roots` are cap-std `Dir` handles opened at construction +/// time (before privilege drop) and used for TOCTOU-safe path +/// resolution on every trigger. +pub struct BinaryHasherCollector { + engine: Arc, + config: BinaryHasherConfig, + opened_roots: Vec, +} + +impl std::fmt::Debug for BinaryHasherCollector { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("BinaryHasherCollector") + .field("config", &self.config) + .field("opened_roots", &self.opened_roots) + .finish_non_exhaustive() + } +} + +impl BinaryHasherCollector { + /// Construct a collector with a shared engine and a validated config. + /// + /// Opens each `allowed_root` as a cap-std `Dir` handle. In strict + /// mode (default), any root that fails to open aborts construction. + /// In partial mode (`allow_partial_roots = true`), failed roots are + /// skipped with a warning. + /// + /// **Lifecycle invariant**: This MUST be called before procmond drops + /// privileges, so the `Dir` handles inherit the elevated fd. + /// + /// # Errors + /// + /// Returns [`TriggerHandleError::Internal`] if `config.validate()` + /// fails, or in strict mode if any root fails to open. + pub fn new( + engine: Arc, + config: BinaryHasherConfig, + ) -> Result { + config.validate()?; + + let mut opened_roots = Vec::with_capacity(config.allowed_roots.len()); + for root_path in &config.allowed_roots { + match AllowedRoot::open(root_path) { + Ok(root) => { + debug!(root = %root_path.display(), "opened allowed root"); + opened_roots.push(root); + } + Err(err) => { + // Strict mode still treats NotFound as non-fatal so + // `with_platform_defaults()` stays portable across CI + // matrices (its rustdoc promises this). Other I/O + // errors (permission denied, etc.) abort construction + // unless partial mode is explicitly enabled. + if config.allow_partial_roots || err.kind() == std::io::ErrorKind::NotFound { + let reason = if err.kind() == std::io::ErrorKind::NotFound { + "root missing" + } else { + "partial mode" + }; + warn!( + root = %root_path.display(), + error = %err, + reason, + "skipping allowed root" + ); + } else { + return Err(TriggerHandleError::Internal(format!( + "failed to open allowed root {}: {err}", + root_path.display() + ))); + } + } + } + } + + if opened_roots.is_empty() { + return Err(TriggerHandleError::Internal( + "no allowed roots could be opened".to_owned(), + )); + } + + Ok(Self { + engine, + config, + opened_roots, + }) + } + + /// Stable collector name used to match `TriggerRequest.target_collector`. + pub const NAME: &'static str = "binary-hasher"; + + // ── Path authorization ────────────────────────────────────────────── + + /// Authorize a request via cap-std `Dir` handles and return the + /// opened file handle together with the path used for recording. + /// + /// **TOCTOU contract**: the returned `std::fs::File` is derived from + /// the cap-std confined open and MUST be passed directly to + /// [`MultiAlgorithmHasher::compute_from_file`]. Callers MUST NOT + /// reopen the returned `PathBuf` by path — doing so reintroduces the + /// authorization-to-hashing TOCTOU window that cap-std was added to + /// close. + /// + /// 1. On macOS, verify all root fingerprints once (before the per-file open). + /// 2. Length cap + parent-traversal rejection (via shared predicates). + /// 3. cap-std confined open (TOCTOU-safe path resolution). + /// 4. File-type check (must be a regular file, via handle fstat). + /// 5. Size check against `config.max_file_size` (via handle fstat). + fn authorize(&self, raw: &str) -> Result<(std::fs::File, PathBuf), TriggerHandleError> { + // Verify all root fingerprints once before entering the per-file + // stream. On non-macOS this is a no-op. + verify_all_fingerprints(&self.opened_roots).map_err(|err| map_auth_err(&err))?; + + if raw.len() > MAX_TARGET_PATH_LEN { + return Err(TriggerHandleError::PathTooLong { len: raw.len() }); + } + if raw.is_empty() { + return Err(TriggerHandleError::PathRejected { + reason: "empty path".to_owned(), + }); + } + + let raw_path = Path::new(raw); + + // Delegate to cap-std authorization. Returns the opened file handle, + // its metadata (via fstat), and the composed (root + relative) path + // we just authorized — so we avoid a second path-based stat. + let (cap_file, meta, resolved) = + authorize_confined_path(raw_path, &self.opened_roots, self.config.follow_symlinks) + .map_err(|err| map_auth_err(&err))?; + + // File-type and size checks use the metadata already obtained from + // the open file handle — no second stat(2) call needed. + if !meta.is_file() { + return Err(TriggerHandleError::Unavailable { + reason: "target is not a regular file".to_owned(), + }); + } + if meta.len() > self.config.max_file_size { + return Err(TriggerHandleError::TooLarge { + size: meta.len(), + limit: self.config.max_file_size, + }); + } + + // Convert the cap-std file handle to a plain `std::fs::File` that + // the engine's blocking reader can consume. The fd is preserved — + // hashing reads the inode that cap-std authorized, not whatever + // `raw_path` might resolve to after authorization. + let file = cap_file.into_std(); + + // For the `HashResult::file_path` record we use the (root + + // relative) path derived at authorization time. This avoids a + // second path-based stat (`canonicalize`) which would reintroduce + // a cosmetic TOCTOU window where `recorded_path` could mismatch + // the inode actually hashed if the path was swapped between the + // cap-std open and the canonicalize. + // SECURITY: this path is NEVER used to reopen the file. The + // engine hashes from `file` (the authorized fd) only. + Ok((file, resolved)) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// TriggerableCollector impl +// ───────────────────────────────────────────────────────────────────────────── + +impl TriggerableCollector for BinaryHasherCollector { + fn name(&self) -> &'static str { + Self::NAME + } + + fn supported_analysis_types(&self) -> &[AnalysisType] { + const TYPES: &[AnalysisType] = &[AnalysisType::BinaryHash]; + TYPES + } + + async fn handle_trigger( + &self, + request: &TriggerRequest, + ) -> Result { + // Audit-log Critical-priority requests regardless of outcome so + // operators can investigate any use of the priority bypass. Note + // that Critical does NOT bypass allowed_roots authorization. + if matches!(request.priority, crate::event::TriggerPriority::Critical) { + info!( + trigger_id = %request.trigger_id, + correlation_id = %request.correlation_id, + target_path = ?request.target_path, + "critical-priority binary-hash trigger accepted for processing" + ); + } + + let raw_path = request + .target_path + .as_deref() + .ok_or(TriggerHandleError::MissingField { + field: "target_path", + })?; + + // Defense-in-depth: the dispatcher already routes by collector + // name, but a direct caller bypassing it could pass any + // `analysis_type`. Reject anything other than `BinaryHash` so we + // never echo a mislabeled payload back to the caller. + // `PathRejected` is used because its `kind()` maps to + // `TriggerErrorKind::InvalidRequest`, the closest wire-level + // semantic to "bad request". + if !matches!(request.analysis_type, AnalysisType::BinaryHash) { + return Err(TriggerHandleError::PathRejected { + reason: format!( + "BinaryHasherCollector received unsupported analysis_type: {:?}", + request.analysis_type + ), + }); + } + + let (file, canonical) = self.authorize(raw_path)?; + + // Delegate to the engine using the authorized file descriptor + // (TOCTOU-safe — the engine reads from the inode we authorized, + // not a path that may have been replaced after cap-std resolution). + // The engine still enforces its own concurrency cap, cache, + // mid-read mutation detection, cooperative cancellation, and timeout. + let hash_result = self + .engine + .compute_from_file(&canonical, file) + .await + .map_err(map_hash_err)?; + + debug!( + trigger_id = %request.trigger_id, + path = %canonical.display(), + algorithms = ?hash_result.hashes.keys().collect::>(), + "binary-hash trigger completed" + ); + + Ok(AnalysisResult { + stage_id: format!("trigger:{}", request.trigger_id), + analysis_type: request.analysis_type.clone(), + collector_id: Self::NAME.to_owned(), + result_data: serde_json::to_value(&hash_result) + .map_err(|e| TriggerHandleError::Internal(format!("serde: {e}")))?, + metadata: HashMap::new(), + completed_at: SystemTime::now(), + execution_duration: hash_result.computation_time, + // Post type-state refactor: a successful `HashResult` is always + // authoritative. Non-authoritative (mid-read mutation) cases are + // returned as `HashError::Nonauthoritative` by the engine and + // mapped to `TriggerErrorKind::Unavailable` via `map_hash_err` + // below, so this arm always observes a stable hash. + confidence: 1.0, + }) + } + + async fn health_check(&self) -> Result<(), TriggerHandleError> { + // Health is good if at least one opened root is accessible. + // Use the cap-std `Dir` handle's `dir_metadata()` for consistency + // with the TOCTOU-safe model — we stat the exact inode opened at + // construction time, not whatever the display path resolves to now. + // Collect per-root failure reasons so the aggregate error is + // actionable; each failing root is also logged individually at + // warn! so operators can identify which roots are inaccessible + // without parsing the combined error message. + let mut failures = Vec::new(); + for root in &self.opened_roots { + match root.handle().dir_metadata() { + Ok(_) => return Ok(()), + Err(err) => { + warn!(root = %root.display_path(), error = %err, "allowed root inaccessible"); + failures.push(format!("{}: {err}", root.display_path())); + } + } + } + Err(TriggerHandleError::Internal(format!( + "no opened_roots accessible: {}", + failures.join(", ") + ))) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Error mapping +// ───────────────────────────────────────────────────────────────────────────── + +/// Map an [`AuthError`] to a wire-safe [`TriggerHandleError`]. +/// +/// Maps path-escape and not-allowed to `PathNotAllowed`, symlink +/// rejection and I/O to `Unavailable`, and everything else to +/// `Internal`. This prevents file-existence oracles. +#[allow( + clippy::wildcard_enum_match_arm, + clippy::pattern_type_mismatch, + reason = "AuthError is #[non_exhaustive]; wildcard required for forward-compat" +)] +fn map_auth_err(err: &AuthError) -> TriggerHandleError { + match err { + AuthError::PathTooLong { len, .. } => TriggerHandleError::PathTooLong { len: *len }, + AuthError::PathTraversal { .. } => TriggerHandleError::PathRejected { + reason: "path contains '..' component".to_owned(), + }, + AuthError::CapStdEscape { .. } | AuthError::PathNotAllowed { .. } => { + TriggerHandleError::PathNotAllowed + } + AuthError::SymlinkRejected { .. } => TriggerHandleError::Unavailable { + reason: "target is a symlink".to_owned(), + }, + AuthError::Io { .. } => TriggerHandleError::Unavailable { + reason: format!("I/O error: {err}"), + }, + // macOS: bind-mount / volume swap since startup. The `kind()` + // mapping sends `Internal` over the wire; we intentionally use a + // generic message here that does NOT leak the affected root path + // (mount topology is sensitive) even though the local + // `TriggerHandleError::Internal` string is not currently exposed + // across the wire boundary. + #[cfg(target_os = "macos")] + AuthError::RootMountChanged { .. } => { + TriggerHandleError::Internal("allowed root mount changed since startup".to_owned()) + } + // Wildcard arm: catches any future AuthError variants added by + // upstream. When new variants are introduced they will silently + // route here as Internal — reviewers MUST add an explicit arm + // above to give them the correct TriggerHandleError mapping. + _ => TriggerHandleError::Internal(format!("auth error: {err}")), + } +} + +/// Map engine-level errors to the wire-safe trait-level error. +/// +/// `PermissionDenied` and `FileNotFound` **both** map to `Unavailable` so a +/// caller cannot use differential error responses to probe the filesystem. +/// +/// The wildcard arm is required because [`HashError`] is `#[non_exhaustive]`; +/// any future variants will transparently fall through to `Internal` until +/// this mapping is explicitly updated. +#[allow( + clippy::wildcard_enum_match_arm, + reason = "HashError is #[non_exhaustive]; wildcard required for forward-compat" +)] +fn map_hash_err(err: HashError) -> TriggerHandleError { + match err { + HashError::PermissionDenied { path } => TriggerHandleError::Unavailable { + reason: format!("permission denied: {}", path.display()), + }, + HashError::FileNotFound { path } => TriggerHandleError::Unavailable { + reason: format!("not found: {}", path.display()), + }, + HashError::FileTooLarge { size, limit } => TriggerHandleError::TooLarge { size, limit }, + HashError::Timeout | HashError::Cancelled => TriggerHandleError::Timeout, + HashError::Io { path, source } => { + TriggerHandleError::Internal(format!("I/O error at {}: {source}", path.display())) + } + HashError::Join(msg) => TriggerHandleError::Internal(format!("join: {msg}")), + HashError::InvalidConfig(msg) => TriggerHandleError::Internal(format!("config: {msg}")), + // A mid-read mutation is a detection signal, not a caller error. + // Map to Unavailable (same bucket as NotFound/PermissionDenied) so + // wire-level responses do not leak the distinction. The audit-level + // telemetry for this event is emitted separately in Phase 4. + HashError::Nonauthoritative { path } => TriggerHandleError::Unavailable { + reason: format!("file mutated during hash: {}", path.display()), + }, + _ => TriggerHandleError::Internal("unrecognized HashError variant".to_owned()), + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Tests +// ───────────────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + + use super::*; + use crate::event::{AnalysisType, TriggerPriority}; + use crate::triggerable::TriggerErrorKind; + use daemoneye_lib::integrity::{HashAlgorithm, HasherConfig}; + use std::collections::HashMap; + use std::fs; + use std::sync::Arc; + use tempfile::{NamedTempFile, TempDir}; + + fn make_engine() -> Arc { + Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()) + } + + fn make_collector(allowed_root: &Path) -> BinaryHasherCollector { + let config = BinaryHasherConfig::default().with_allowed_root(allowed_root); + BinaryHasherCollector::new(make_engine(), config).unwrap() + } + + fn make_request(path: impl Into) -> TriggerRequest { + TriggerRequest { + trigger_id: "t-test".to_owned(), + target_collector: BinaryHasherCollector::NAME.to_owned(), + analysis_type: AnalysisType::BinaryHash, + priority: TriggerPriority::Normal, + target_pid: None, + target_path: Some(path.into()), + correlation_id: "c-test".to_owned(), + metadata: HashMap::new(), + timestamp: SystemTime::now(), + } + } + + // ── Config validation ─────────────────────────────────────────────── + + #[test] + fn empty_allowed_roots_deny_on_empty() { + let config = BinaryHasherConfig::default(); + let err = BinaryHasherCollector::new(make_engine(), config).unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::Internal); + } + + #[test] + fn platform_defaults_populate_allowed_roots() { + let config = BinaryHasherConfig::default().with_platform_defaults(); + assert!( + !config.allowed_roots.is_empty(), + "platform defaults should produce a non-empty allow-list" + ); + } + + #[test] + fn zero_max_file_size_fails_validation() { + let config = BinaryHasherConfig::default() + .with_allowed_root("/tmp") + .with_max_file_size(0); + assert!(config.validate().is_err()); + } + + #[test] + fn zero_timeout_fails_validation() { + let config = BinaryHasherConfig::default() + .with_allowed_root("/tmp") + .with_timeout(Duration::ZERO); + assert!(config.validate().is_err()); + } + + // ── Successful happy path ─────────────────────────────────────────── + + #[tokio::test] + async fn hashes_file_under_allowed_root() { + let dir = TempDir::new().unwrap(); + let file_path = dir.path().join("hello.bin"); + fs::write(&file_path, b"hello binary hasher").unwrap(); + let collector = make_collector(dir.path()); + let req = make_request(file_path.to_string_lossy().into_owned()); + let result = collector.handle_trigger(&req).await.unwrap(); + assert_eq!(result.collector_id, "binary-hasher"); + assert_eq!(result.analysis_type, AnalysisType::BinaryHash); + // result_data should contain the hex hashes. + let hashes = result.result_data.get("hashes").unwrap(); + let sha256 = hashes.get(HashAlgorithm::Sha256.wire_name()).unwrap(); + assert_eq!(sha256.as_str().unwrap().len(), 64); + } + + // ── Security: allowed_roots enforcement ───────────────────────────── + + #[tokio::test] + async fn rejects_path_outside_allowed_roots() { + let allowed = TempDir::new().unwrap(); + let other = TempDir::new().unwrap(); + let file = other.path().join("unauthorized.bin"); + fs::write(&file, b"outside").unwrap(); + + let collector = make_collector(allowed.path()); + let req = make_request(file.to_string_lossy().into_owned()); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::PathNotAllowed); + } + + // ── Security: path length cap ─────────────────────────────────────── + + #[tokio::test] + async fn rejects_over_length_path_before_io() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + let oversized = "a".repeat(MAX_TARGET_PATH_LEN + 1); + let req = make_request(oversized); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::InvalidRequest); + } + + #[tokio::test] + async fn rejects_empty_path() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + let req = make_request(""); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::InvalidRequest); + } + + // ── Security: parent-traversal rejection ──────────────────────────── + + #[tokio::test] + async fn rejects_path_with_parent_component() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + let attack = format!("{}/../../etc/passwd", dir.path().display()); + let req = make_request(attack); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::InvalidRequest); + } + + // ── Security: symlink rejection (default) ─────────────────────────── + + #[cfg(unix)] + #[tokio::test] + async fn rejects_symlink_target_by_default() { + let dir = TempDir::new().unwrap(); + let real = dir.path().join("real.bin"); + fs::write(&real, b"content").unwrap(); + let link = dir.path().join("link.bin"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + let collector = make_collector(dir.path()); + let req = make_request(link.to_string_lossy().into_owned()); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::Unavailable); + } + + #[cfg(unix)] + #[tokio::test] + async fn follows_symlink_when_opted_in() { + let dir = TempDir::new().unwrap(); + let real = dir.path().join("real.bin"); + fs::write(&real, b"content").unwrap(); + let link = dir.path().join("link.bin"); + // Use a relative symlink so cap-std doesn't reject it as an + // escape (absolute symlink targets point outside the Dir sandbox). + std::os::unix::fs::symlink(Path::new("real.bin"), &link).unwrap(); + + let config = BinaryHasherConfig::default() + .with_allowed_root(dir.path()) + .with_follow_symlinks(true); + let collector = BinaryHasherCollector::new(make_engine(), config).unwrap(); + let req = make_request(link.to_string_lossy().into_owned()); + let result = collector.handle_trigger(&req).await.unwrap(); + assert_eq!(result.collector_id, "binary-hasher"); + } + + // ── Security: file-existence oracle defense ───────────────────────── + + #[tokio::test] + async fn not_found_and_denied_both_map_to_unavailable() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + + let missing = dir.path().join("does-not-exist.bin"); + let req = make_request(missing.to_string_lossy().into_owned()); + let err = collector.handle_trigger(&req).await.unwrap_err(); + // Not-found path is Unavailable on the wire. + assert_eq!(err.kind(), TriggerErrorKind::Unavailable); + } + + // ── Security: oversized file rejection ────────────────────────────── + + #[tokio::test] + async fn oversized_file_rejected_before_hashing() { + let dir = TempDir::new().unwrap(); + let file = dir.path().join("big.bin"); + fs::write(&file, vec![0_u8; 8192]).unwrap(); + let config = BinaryHasherConfig::default() + .with_allowed_root(dir.path()) + .with_max_file_size(1024); + let collector = BinaryHasherCollector::new(make_engine(), config).unwrap(); + let req = make_request(file.to_string_lossy().into_owned()); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::TooLarge); + } + + // ── MissingField ──────────────────────────────────────────────────── + + #[tokio::test] + async fn missing_target_path_is_invalid_request() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + let mut req = make_request("ignored"); + req.target_path = None; + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::InvalidRequest); + } + + // ── Defense-in-depth: analysis_type validation ────────────────────── + + /// A direct caller bypassing the dispatcher must not be able to get + /// the collector to echo a mislabeled [`AnalysisResult`] by passing + /// a non-[`AnalysisType::BinaryHash`] value. The collector must + /// reject the request before doing any I/O or hashing. + #[tokio::test] + async fn handle_trigger_rejects_non_binary_hash_analysis_type() { + let dir = TempDir::new().unwrap(); + let file_path = dir.path().join("hello.bin"); + fs::write(&file_path, b"hello binary hasher").unwrap(); + + let collector = make_collector(dir.path()); + let mut req = make_request(file_path.to_string_lossy().into_owned()); + req.analysis_type = AnalysisType::YaraScan; + + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::InvalidRequest); + } + + // ── Directories rejected ──────────────────────────────────────────── + + #[tokio::test] + async fn directory_target_is_unavailable() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + let req = make_request(dir.path().to_string_lossy().into_owned()); + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!(err.kind(), TriggerErrorKind::Unavailable); + } + + // ── Trait metadata ────────────────────────────────────────────────── + + #[test] + fn collector_name_matches_trigger_request_target() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + assert_eq!(collector.name(), "binary-hasher"); + assert_eq!( + collector.supported_analysis_types(), + &[AnalysisType::BinaryHash] + ); + } + + #[tokio::test] + async fn health_check_passes_with_reachable_root() { + let dir = TempDir::new().unwrap(); + let collector = make_collector(dir.path()); + collector.health_check().await.unwrap(); + } + + // ── Critical priority is queue-order only ─────────────────────────── + + #[tokio::test] + async fn critical_priority_still_respects_allowed_roots() { + let allowed = TempDir::new().unwrap(); + let outside = TempDir::new().unwrap(); + let file = outside.path().join("critical.bin"); + fs::write(&file, b"content").unwrap(); + + let collector = make_collector(allowed.path()); + let mut req = make_request(file.to_string_lossy().into_owned()); + req.priority = TriggerPriority::Critical; + let err = collector.handle_trigger(&req).await.unwrap_err(); + assert_eq!( + err.kind(), + TriggerErrorKind::PathNotAllowed, + "Critical priority must NOT bypass allowed_roots authorization" + ); + } + + // ── Keep _tmp alive to prove fn signature is usable ───────────────── + + #[test] + fn named_temp_file_compiles() { + // Sanity check that the dev-dep feature is available. + let _tmp: NamedTempFile = NamedTempFile::new().unwrap(); + } + + // ── Regression: cap-std escape message stays stable across upgrades ── + + /// Regression test for [`CAP_STD_ESCAPE_MESSAGE`]. + /// + /// Opens an [`AllowedRoot`] on a tempdir, then calls + /// `root.handle().open("../../../etc/hosts")` directly — bypassing + /// `check_no_traversal` — to exercise the live cap-std confinement + /// boundary. The returned error message MUST contain + /// [`CAP_STD_ESCAPE_MESSAGE`]; if it does not, cap-std changed its + /// error text and [`authorize_confined_path`] will silently stop + /// mapping escapes to [`AuthError::CapStdEscape`]. + #[test] + fn cap_std_escape_error_contains_expected_message() { + let dir = TempDir::new().unwrap(); + let root = AllowedRoot::open(dir.path()).expect("should open tempdir as AllowedRoot"); + + // Attempt a path that escapes the sandbox. `check_no_traversal` + // is NOT called here so we reach cap-std's own escape detection. + let result = root.handle().open("../../../etc/hosts"); + let err = result.expect_err("cap-std should reject path escaping the Dir sandbox"); + let msg = err.to_string(); + assert!( + msg.contains(CAP_STD_ESCAPE_MESSAGE), + "cap-std escape error message changed — update CAP_STD_ESCAPE_MESSAGE.\n\ + Expected to contain: {CAP_STD_ESCAPE_MESSAGE:?}\n\ + Got: {msg:?}" + ); + } +} diff --git a/collector-core/src/collector.rs b/collector-core/src/collector.rs index 66d1ed53..1dceac33 100644 --- a/collector-core/src/collector.rs +++ b/collector-core/src/collector.rs @@ -2030,6 +2030,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/src/daemoneye_event_bus.rs b/collector-core/src/daemoneye_event_bus.rs index 508bcf5b..e1c0627f 100644 --- a/collector-core/src/daemoneye_event_bus.rs +++ b/collector-core/src/daemoneye_event_bus.rs @@ -498,6 +498,7 @@ impl DaemoneyeEventBus { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1211,6 +1212,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -1366,6 +1368,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1463,6 +1466,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1529,6 +1533,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1590,6 +1595,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1724,6 +1730,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1796,6 +1803,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1877,6 +1885,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/src/event.rs b/collector-core/src/event.rs index 80ad8af9..bec8a207 100644 --- a/collector-core/src/event.rs +++ b/collector-core/src/event.rs @@ -39,6 +39,7 @@ use std::time::SystemTime; /// cpu_usage: None, /// memory_usage: None, /// executable_hash: None, +/// hash_algorithm: None, /// user_id: None, /// accessible: true, /// file_exists: true, @@ -102,9 +103,30 @@ pub struct ProcessEvent { /// Memory usage in bytes pub memory_usage: Option, - /// SHA-256 hash of executable + /// Hex-encoded cryptographic hash of the executable file. Populated + /// when the collector was constructed with + /// `compute_executable_hashes = true` and the executable was readable. + /// See [`Self::hash_algorithm`] for the algorithm that produced this + /// value. + /// + /// `None` means one of: + /// - Hashing was disabled at the collector level + /// - The executable was inaccessible, deleted, oversized, or failed to + /// open for reading + /// - Hash computation timed out or failed + /// + /// Consumers **must** compare `(executable_hash, hash_algorithm)` as a + /// tuple when checking for lifecycle drift. Comparing only the hex + /// string would silently alias if the canonical algorithm changes + /// across procmond versions. pub executable_hash: Option, + /// Canonical lowercase name of the algorithm used for + /// [`Self::executable_hash`] (e.g. `"sha256"`, `"blake3"`). Always + /// `None` when `executable_hash` is `None`; always `Some` when it is + /// populated. + pub hash_algorithm: Option, + /// User ID running the process pub user_id: Option, @@ -362,6 +384,26 @@ pub enum TriggerPriority { Critical, } +impl ProcessEvent { + /// Validate the `(executable_hash, hash_algorithm)` paired + /// invariant: both fields must be `Some` together or `None` + /// together. A mismatch indicates wire-format drift or a buggy + /// constructor and should be treated as data corruption by + /// downstream consumers. + /// + /// # Errors + /// + /// Returns `Err` with a human-readable message if the invariant + /// is violated. + pub const fn validate_hash_tuple(&self) -> Result<(), &'static str> { + if self.executable_hash.is_some() != self.hash_algorithm.is_some() { + return Err("ProcessEvent invariant violated: executable_hash and \ + hash_algorithm must both be Some or both None"); + } + Ok(()) + } +} + impl CollectionEvent { /// Returns the timestamp of the event regardless of type. pub const fn timestamp(&self) -> SystemTime { @@ -452,6 +494,7 @@ mod tests { cpu_usage: Some(5.5), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -466,6 +509,90 @@ mod tests { assert!(event.file_exists); } + #[test] + fn test_validate_hash_tuple_inconsistent_rejected() { + let timestamp = SystemTime::now(); + // hash without algorithm -> invariant violated + let bad_hash_only = ProcessEvent { + pid: 1, + ppid: None, + name: "bad".to_owned(), + executable_path: None, + command_line: vec![], + start_time: None, + cpu_usage: None, + memory_usage: None, + executable_hash: Some("abc".to_owned()), + hash_algorithm: None, + user_id: None, + accessible: true, + file_exists: true, + timestamp, + platform_metadata: None, + }; + assert!(bad_hash_only.validate_hash_tuple().is_err()); + + // algorithm without hash -> invariant violated + let bad_algo_only = ProcessEvent { + pid: 2, + ppid: None, + name: "bad".to_owned(), + executable_path: None, + command_line: vec![], + start_time: None, + cpu_usage: None, + memory_usage: None, + executable_hash: None, + hash_algorithm: Some("sha256".to_owned()), + user_id: None, + accessible: true, + file_exists: true, + timestamp, + platform_metadata: None, + }; + assert!(bad_algo_only.validate_hash_tuple().is_err()); + + // both None -> valid + let neither = ProcessEvent { + pid: 3, + ppid: None, + name: "ok".to_owned(), + executable_path: None, + command_line: vec![], + start_time: None, + cpu_usage: None, + memory_usage: None, + executable_hash: None, + hash_algorithm: None, + user_id: None, + accessible: true, + file_exists: true, + timestamp, + platform_metadata: None, + }; + assert!(neither.validate_hash_tuple().is_ok()); + + // both Some -> valid + let both = ProcessEvent { + pid: 4, + ppid: None, + name: "ok".to_owned(), + executable_path: None, + command_line: vec![], + start_time: None, + cpu_usage: None, + memory_usage: None, + executable_hash: Some("abc".to_owned()), + hash_algorithm: Some("sha256".to_owned()), + user_id: None, + accessible: true, + file_exists: true, + timestamp, + platform_metadata: None, + }; + assert!(both.validate_hash_tuple().is_ok()); + } + #[test] fn test_collection_event_timestamp() { let timestamp = SystemTime::now(); @@ -479,6 +606,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -588,6 +716,7 @@ mod tests { cpu_usage: Some(1.5), memory_usage: Some(4096), executable_hash: Some("hash123".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, diff --git a/collector-core/src/event_bus.rs b/collector-core/src/event_bus.rs index 51ad8c21..7e7e17f8 100644 --- a/collector-core/src/event_bus.rs +++ b/collector-core/src/event_bus.rs @@ -1541,6 +1541,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -1611,6 +1612,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -1787,6 +1789,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1912,6 +1915,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1992,6 +1996,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -2084,6 +2089,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -2296,6 +2302,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -2314,6 +2321,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -2429,6 +2437,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -2545,6 +2554,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/src/high_performance_event_bus.rs b/collector-core/src/high_performance_event_bus.rs index b85fa67b..a28e7aa8 100644 --- a/collector-core/src/high_performance_event_bus.rs +++ b/collector-core/src/high_performance_event_bus.rs @@ -771,6 +771,7 @@ mod tests { cpu_usage: Some(0.5), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, diff --git a/collector-core/src/lib.rs b/collector-core/src/lib.rs index 36b64847..f806a2fa 100644 --- a/collector-core/src/lib.rs +++ b/collector-core/src/lib.rs @@ -63,6 +63,7 @@ //! ``` pub mod analysis_chain; +pub mod binary_hasher; pub mod capability_router; pub mod collector; pub mod config; @@ -83,6 +84,7 @@ pub mod shutdown_coordinator; pub mod source; pub mod task_distributor; pub mod trigger; +pub mod triggerable; // Re-export main types for convenience pub use analysis_chain::{ @@ -90,6 +92,7 @@ pub use analysis_chain::{ AnalysisWorkflowDefinition, StageStatus, WorkflowError, WorkflowErrorType, WorkflowExecution, WorkflowProgress, WorkflowStatistics, WorkflowStatus, }; +pub use binary_hasher::{BinaryHasherCollector, BinaryHasherConfig, MAX_TARGET_PATH_LEN}; pub use capability_router::{ CapabilityRouter, CollectorCapability, CollectorHealthStatus, RoutingDecision, RoutingStats, }; @@ -141,3 +144,4 @@ pub use trigger::{ TriggerCapabilities, TriggerCondition, TriggerConfig, TriggerEmissionStats, TriggerManager, TriggerResourceLimits, TriggerStatistics, }; +pub use triggerable::{TriggerErrorKind, TriggerHandleError, TriggerableCollector}; diff --git a/collector-core/src/performance.rs b/collector-core/src/performance.rs index fb71c2e4..5d5c14dc 100644 --- a/collector-core/src/performance.rs +++ b/collector-core/src/performance.rs @@ -956,6 +956,7 @@ mod tests { cpu_usage: Some(1.5), memory_usage: Some(1024 * 1024), executable_hash: Some("test_hash".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -1092,6 +1093,7 @@ mod tests { cpu_usage: Some(1.5), memory_usage: Some(1024 * 1024), executable_hash: Some("test_hash".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, diff --git a/collector-core/src/result_aggregator.rs b/collector-core/src/result_aggregator.rs index b68ebda3..2c4e43d7 100644 --- a/collector-core/src/result_aggregator.rs +++ b/collector-core/src/result_aggregator.rs @@ -593,6 +593,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -617,6 +618,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -661,6 +663,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/src/task_distributor.rs b/collector-core/src/task_distributor.rs index ddff46e1..8fbbcdc3 100644 --- a/collector-core/src/task_distributor.rs +++ b/collector-core/src/task_distributor.rs @@ -626,6 +626,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/src/triggerable.rs b/collector-core/src/triggerable.rs new file mode 100644 index 00000000..942d06c0 --- /dev/null +++ b/collector-core/src/triggerable.rs @@ -0,0 +1,411 @@ +//! Triggered collector framework. +//! +//! Defines the [`TriggerableCollector`] trait — a reactive collector that +//! responds to [`TriggerRequest`] events rather than producing events on its +//! own schedule like a [`crate::source::EventSource`] does. Triggered collectors are the +//! extension point for on-demand analysis collectors such as the binary +//! hasher, YARA scanner, or memory analyzer. +//! +//! # Why not extend `EventSource`? +//! +//! `EventSource::start()` takes an `mpsc::Sender` and a +//! shutdown signal — it models a long-running producer on its own schedule. +//! A triggered collector is reactive: it waits for a `TriggerRequest` and +//! produces an `AnalysisResult`. Forcing an `EventSource` supertrait would +//! make `start()` a dummy sleep loop burning a worker task. +//! +//! Instead, the runtime holds a parallel registry keyed by +//! [`TriggerableCollector::name`]. Dispatch is driven by the existing +//! [`crate::trigger::TriggerManager`] when a matching `TriggerRequest` flows +//! through the bus. +//! +//! # Response routing +//! +//! Responses flow via `oneshot::Sender>` held by +//! the dispatcher, **not** as a new [`crate::event::CollectionEvent`] variant. This: +//! +//! - Avoids polluting the fan-out event bus with responses that only matter +//! to the original requester. +//! - Reuses the existing [`crate::analysis_chain::AnalysisResult`] type +//! instead of duplicating it. +//! - Matches the precedent set by +//! [`crate::analysis_chain::AnalysisChainCoordinator`]. +//! +//! # Error sanitization +//! +//! Triggered collectors may produce rich internal errors (file paths, OS +//! error messages, syscall failures). When a response crosses any process +//! or serialization boundary, those internals must be mapped to the closed +//! [`TriggerErrorKind`] enum so they cannot leak filesystem layout, user +//! data, or file-existence oracles to unauthorized subscribers. See +//! `docs/solutions/security-issues/binary-hashing-authorization-and-toctou-fixes.md` +//! for the full security model and trade-off rationale. + +use crate::analysis_chain::AnalysisResult; +use crate::event::{AnalysisType, TriggerRequest}; +use serde::{Deserialize, Serialize}; +use std::fmt; +use thiserror::Error; + +// ───────────────────────────────────────────────────────────────────────────── +// TriggerErrorKind (wire-safe, closed enum) +// ───────────────────────────────────────────────────────────────────────────── + +/// Wire-safe error kinds for triggered analysis responses. +/// +/// Rich internal errors (with paths, OS error text, etc.) are mapped to this +/// closed enum before crossing any serialization or process boundary. The +/// mapping intentionally merges [`TriggerErrorKind::Unavailable`] for both +/// "file not found" and "permission denied" so a caller cannot use +/// differential error responses as a file-existence oracle. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[non_exhaustive] +#[serde(rename_all = "snake_case")] +pub enum TriggerErrorKind { + /// The target was not reachable. Covers both permission-denied and + /// not-found to prevent file-existence oracles. + Unavailable, + /// The target exceeded a size limit enforced by the collector. + TooLarge, + /// The request deadline expired before completion. + Timeout, + /// The target path was not in the collector's allow-list. + PathNotAllowed, + /// The collector's resource budget was saturated. + ResourceExhausted, + /// The request was invalid (missing fields, bad length, etc.). + InvalidRequest, + /// An internal error occurred. Details are logged locally at `warn` + /// level but not exposed on the wire. + Internal, +} + +impl fmt::Display for TriggerErrorKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match *self { + Self::Unavailable => "unavailable", + Self::TooLarge => "too_large", + Self::Timeout => "timeout", + Self::PathNotAllowed => "path_not_allowed", + Self::ResourceExhausted => "resource_exhausted", + Self::InvalidRequest => "invalid_request", + Self::Internal => "internal", + }; + f.write_str(name) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// TriggerHandleError (internal, rich context) +// ───────────────────────────────────────────────────────────────────────────── + +/// Rich internal error from [`TriggerableCollector::handle_trigger`]. +/// +/// This type carries full context (paths, OS errors, messages) for local +/// `tracing` logs and test assertions. It **must** be converted to +/// [`TriggerErrorKind`] via [`TriggerHandleError::kind`] before any +/// serialization or cross-boundary transmission. +#[derive(Debug, Error)] +#[non_exhaustive] +pub enum TriggerHandleError { + /// The request was missing a required field. + #[error("trigger request missing field: {field}")] + MissingField { + /// Name of the missing field. + field: &'static str, + }, + /// The request path was longer than the collector's hard limit. + #[error("trigger request path too long ({len} bytes)")] + PathTooLong { + /// Observed path length in bytes. + len: usize, + }, + /// The request contained a malformed or rejected path. + #[error("trigger request path rejected: {reason}")] + PathRejected { + /// Why the path was rejected (logged locally, not on the wire). + reason: String, + }, + /// The target path was not inside any of the collector's allowed roots. + #[error("target path not in allowed roots")] + PathNotAllowed, + /// The target could not be reached (permission denied or not found). + /// Merged on the wire to prevent file-existence oracles. + #[error("target unavailable: {reason}")] + Unavailable { + /// Reason (logged locally, not on the wire). + reason: String, + }, + /// The target exceeded the collector's size limit. + #[error("target too large: {size} bytes (limit {limit})")] + TooLarge { + /// Observed size. + size: u64, + /// Configured limit. + limit: u64, + }, + /// The request deadline expired. + #[error("trigger handler timed out")] + Timeout, + /// The collector's resource budget was saturated. + #[error("collector resource budget exhausted")] + ResourceExhausted, + /// An unexpected error occurred inside the collector. + #[error("internal error: {0}")] + Internal(String), +} + +impl TriggerHandleError { + /// Map this rich internal error to a wire-safe closed-enum kind. + /// + /// `PermissionDenied`/`NotFound`-style variants both map to + /// [`TriggerErrorKind::Unavailable`] to prevent file-existence oracles. + #[must_use] + pub const fn kind(&self) -> TriggerErrorKind { + match *self { + Self::MissingField { .. } | Self::PathRejected { .. } => { + TriggerErrorKind::InvalidRequest + } + Self::PathTooLong { .. } => TriggerErrorKind::InvalidRequest, + Self::PathNotAllowed => TriggerErrorKind::PathNotAllowed, + Self::Unavailable { .. } => TriggerErrorKind::Unavailable, + Self::TooLarge { .. } => TriggerErrorKind::TooLarge, + Self::Timeout => TriggerErrorKind::Timeout, + Self::ResourceExhausted => TriggerErrorKind::ResourceExhausted, + Self::Internal(_) => TriggerErrorKind::Internal, + } + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// TriggerableCollector trait +// ───────────────────────────────────────────────────────────────────────────── + +/// A reactive collector that produces [`AnalysisResult`]s in response to +/// [`TriggerRequest`]s. +/// +/// Unlike [`crate::source::EventSource`], a `TriggerableCollector` does not +/// own a long-running task. The runtime's +/// [`crate::trigger::TriggerManager`] dispatches a `TriggerRequest` to the +/// collector whose [`Self::name`] matches +/// [`TriggerRequest::target_collector`], then delivers the result via a +/// `oneshot::Sender` held by the dispatcher. +/// +/// Workspace convention: use native `async fn` with +/// `#[allow(async_fn_in_trait)]` (matches [`crate::source::EventSource`] and +/// [`crate::monitor_collector::MonitorCollector`]). +#[allow(async_fn_in_trait)] +pub trait TriggerableCollector: Send + Sync { + /// Stable name of this collector. Must match + /// [`TriggerRequest::target_collector`] exactly for the dispatcher to + /// route to this instance. + fn name(&self) -> &'static str; + + /// Analysis types this collector can handle. The dispatcher consults + /// this list for validation before calling [`Self::handle_trigger`]. + fn supported_analysis_types(&self) -> &[AnalysisType]; + + /// Handle a single trigger request. Implementations must respect the + /// request's deadline (derived from the `TriggerManager`'s + /// configuration) and produce a well-formed [`AnalysisResult`] on + /// success or a [`TriggerHandleError`] on failure. + /// + /// # Errors + /// + /// Returns [`TriggerHandleError`] for authorization failures, resource + /// exhaustion, I/O errors, timeout, or invalid input. Rich errors are + /// logged locally; only the result of [`TriggerHandleError::kind`] + /// crosses any serialization boundary. + async fn handle_trigger( + &self, + request: &TriggerRequest, + ) -> Result; + + /// Health check for this collector. Default implementation returns + /// `Ok(())`. + /// + /// # Errors + /// + /// Implementations may return [`TriggerHandleError::Internal`] to + /// signal unhealthy state. + async fn health_check(&self) -> Result<(), TriggerHandleError> { + Ok(()) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Tests +// ───────────────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + + use super::*; + use crate::event::{AnalysisType, TriggerPriority, TriggerRequest}; + use std::collections::HashMap; + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::time::{Duration, SystemTime}; + + // ── TriggerErrorKind serialization stability ──────────────────────── + + #[test] + fn trigger_error_kind_display_is_stable() { + assert_eq!(TriggerErrorKind::Unavailable.to_string(), "unavailable"); + assert_eq!(TriggerErrorKind::TooLarge.to_string(), "too_large"); + assert_eq!(TriggerErrorKind::Timeout.to_string(), "timeout"); + assert_eq!( + TriggerErrorKind::PathNotAllowed.to_string(), + "path_not_allowed" + ); + assert_eq!( + TriggerErrorKind::ResourceExhausted.to_string(), + "resource_exhausted" + ); + assert_eq!( + TriggerErrorKind::InvalidRequest.to_string(), + "invalid_request" + ); + assert_eq!(TriggerErrorKind::Internal.to_string(), "internal"); + } + + #[test] + fn trigger_error_kind_serializes_as_snake_case() { + let json = serde_json::to_string(&TriggerErrorKind::PathNotAllowed).unwrap(); + assert_eq!(json, "\"path_not_allowed\""); + } + + // ── PermissionDenied + NotFound merge to Unavailable ──────────────── + + #[test] + fn permission_denied_and_not_found_both_map_to_unavailable() { + // Both "file not found" style and "permission denied" style errors + // MUST map to the same wire kind. This is the file-existence oracle + // defense required by the security review (H3) in the plan. + let not_found = TriggerHandleError::Unavailable { + reason: "file not found".to_owned(), + }; + let perm_denied = TriggerHandleError::Unavailable { + reason: "permission denied".to_owned(), + }; + assert_eq!(not_found.kind(), TriggerErrorKind::Unavailable); + assert_eq!(perm_denied.kind(), TriggerErrorKind::Unavailable); + assert_eq!(not_found.kind(), perm_denied.kind()); + } + + #[test] + fn error_kinds_round_trip_through_wire_mapping() { + let cases: Vec<(TriggerHandleError, TriggerErrorKind)> = vec![ + ( + TriggerHandleError::MissingField { field: "path" }, + TriggerErrorKind::InvalidRequest, + ), + ( + TriggerHandleError::PathTooLong { len: 9999 }, + TriggerErrorKind::InvalidRequest, + ), + ( + TriggerHandleError::PathNotAllowed, + TriggerErrorKind::PathNotAllowed, + ), + ( + TriggerHandleError::TooLarge { + size: 2_000, + limit: 1_000, + }, + TriggerErrorKind::TooLarge, + ), + (TriggerHandleError::Timeout, TriggerErrorKind::Timeout), + ( + TriggerHandleError::ResourceExhausted, + TriggerErrorKind::ResourceExhausted, + ), + ( + TriggerHandleError::Internal("boom".to_owned()), + TriggerErrorKind::Internal, + ), + ]; + for (err, expected) in cases { + assert_eq!(err.kind(), expected); + } + } + + // ── Trait contract test using an in-memory mock ───────────────────── + + fn sample_trigger(target: &str) -> TriggerRequest { + TriggerRequest { + trigger_id: "t-1".to_owned(), + target_collector: target.to_owned(), + analysis_type: AnalysisType::BinaryHash, + priority: TriggerPriority::Normal, + target_pid: Some(1234), + target_path: Some("/usr/bin/ls".to_owned()), + correlation_id: "c-1".to_owned(), + metadata: HashMap::new(), + timestamp: SystemTime::now(), + } + } + + struct CountingCollector { + name: &'static str, + calls: Arc, + } + + impl TriggerableCollector for CountingCollector { + fn name(&self) -> &'static str { + self.name + } + + fn supported_analysis_types(&self) -> &[AnalysisType] { + const TYPES: &[AnalysisType] = &[AnalysisType::BinaryHash]; + TYPES + } + + async fn handle_trigger( + &self, + request: &TriggerRequest, + ) -> Result { + let _ = self.calls.fetch_add(1, Ordering::Relaxed); + Ok(AnalysisResult { + stage_id: "mock-stage".to_owned(), + analysis_type: request.analysis_type.clone(), + collector_id: self.name.to_owned(), + result_data: serde_json::json!({"mock": true}), + metadata: HashMap::new(), + completed_at: SystemTime::now(), + execution_duration: Duration::from_millis(1), + confidence: 1.0, + }) + } + } + + #[tokio::test] + async fn collector_handles_trigger_and_increments_counter() { + let calls = Arc::new(AtomicUsize::new(0)); + let collector = CountingCollector { + name: "mock-hasher", + calls: Arc::clone(&calls), + }; + assert_eq!(collector.name(), "mock-hasher"); + assert_eq!( + collector.supported_analysis_types(), + &[AnalysisType::BinaryHash] + ); + + let req = sample_trigger("mock-hasher"); + let result = collector.handle_trigger(&req).await.unwrap(); + assert_eq!(result.collector_id, "mock-hasher"); + assert_eq!(result.analysis_type, AnalysisType::BinaryHash); + assert_eq!(calls.load(Ordering::Relaxed), 1); + } + + #[tokio::test] + async fn default_health_check_is_ok() { + let collector = CountingCollector { + name: "mock", + calls: Arc::new(AtomicUsize::new(0)), + }; + collector.health_check().await.unwrap(); + } +} diff --git a/collector-core/tests/chaos_testing.rs b/collector-core/tests/chaos_testing.rs index 1e3da1d3..9adc951b 100644 --- a/collector-core/tests/chaos_testing.rs +++ b/collector-core/tests/chaos_testing.rs @@ -267,7 +267,8 @@ impl EventSource for ChaosEventSource { start_time: Some(SystemTime::now()), cpu_usage: Some(50.0), memory_usage: Some(1024 * 1024 * 100), // 100MB - executable_hash: Some("a".repeat(64)), // Large hash + executable_hash: Some("a".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), // Large hash user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -287,6 +288,7 @@ impl EventSource for ChaosEventSource { cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("chaos_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/compatibility_integration_test.rs b/collector-core/tests/compatibility_integration_test.rs index 3f6b5ca0..de7fc8e1 100644 --- a/collector-core/tests/compatibility_integration_test.rs +++ b/collector-core/tests/compatibility_integration_test.rs @@ -124,6 +124,7 @@ impl LegacyCompatibleSource { cpu_usage: Some((event_id % 100) as f64), memory_usage: Some(1024 * (event_id + 1) as u64), executable_hash: Some(format!("legacy_hash_{:064x}", event_id)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -173,6 +174,7 @@ impl EventSource for LegacyCompatibleSource { cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some(format!("modern_hash_{:064x}", event_count)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/comprehensive_test_suite.rs b/collector-core/tests/comprehensive_test_suite.rs index 5b88b0e7..a6baa5bd 100644 --- a/collector-core/tests/comprehensive_test_suite.rs +++ b/collector-core/tests/comprehensive_test_suite.rs @@ -212,6 +212,7 @@ impl EventSource for ComprehensiveTestSource { cpu_usage: Some(1.0 + (i as f64 * 0.1) % 10.0), memory_usage: Some(1024 * 1024 + (i as u64 * 1024)), executable_hash: Some(format!("hash_{:08x}", i)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/concurrent_lifecycle_test.rs b/collector-core/tests/concurrent_lifecycle_test.rs index 91ec530a..dcba41b0 100644 --- a/collector-core/tests/concurrent_lifecycle_test.rs +++ b/collector-core/tests/concurrent_lifecycle_test.rs @@ -171,6 +171,7 @@ impl EventSource for LifecycleTestSource { cpu_usage: Some(0.5), memory_usage: Some(512 * 1024), executable_hash: Some("lifecycle_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/daemoneye_eventbus_integration.rs b/collector-core/tests/daemoneye_eventbus_integration.rs index f11d794f..a2cf79b4 100644 --- a/collector-core/tests/daemoneye_eventbus_integration.rs +++ b/collector-core/tests/daemoneye_eventbus_integration.rs @@ -109,6 +109,7 @@ async fn test_daemoneye_eventbus_integration() { cpu_usage: Some(5.5), memory_usage: Some(1024 * 1024), // 1MB executable_hash: Some("abc123def456".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -286,6 +287,7 @@ async fn test_daemoneye_eventbus_multiple_subscribers() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -399,6 +401,7 @@ async fn test_daemoneye_eventbus_performance_comparison() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/tests/daemoneye_eventbus_ipc_integration.rs b/collector-core/tests/daemoneye_eventbus_ipc_integration.rs index 92d48df8..417911c2 100644 --- a/collector-core/tests/daemoneye_eventbus_ipc_integration.rs +++ b/collector-core/tests/daemoneye_eventbus_ipc_integration.rs @@ -139,6 +139,7 @@ async fn test_daemoneye_eventbus_vs_local_eventbus_performance() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -346,6 +347,7 @@ async fn test_eventbus_broker_access_integration() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -432,6 +434,7 @@ async fn test_eventbus_seamless_migration_compatibility() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/daemoneye_eventbus_monitoring_integration.rs b/collector-core/tests/daemoneye_eventbus_monitoring_integration.rs index c86cd430..6a7f8a25 100644 --- a/collector-core/tests/daemoneye_eventbus_monitoring_integration.rs +++ b/collector-core/tests/daemoneye_eventbus_monitoring_integration.rs @@ -108,6 +108,7 @@ async fn test_daemoneye_eventbus_monitoring_integration() { cpu_usage: Some(10.0 + i as f64), memory_usage: Some(1024 * (i + 1) as u64), executable_hash: Some(format!("hash_{}", i)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -218,6 +219,7 @@ async fn test_monitoring_metrics_persistence() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/tests/eventbus_migration_comparison.rs b/collector-core/tests/eventbus_migration_comparison.rs index 4b2f86e0..e35e0387 100644 --- a/collector-core/tests/eventbus_migration_comparison.rs +++ b/collector-core/tests/eventbus_migration_comparison.rs @@ -109,6 +109,7 @@ impl SimpleEventGenerator { cpu_usage: Some(10.0 + id as f64), memory_usage: Some(1024 * (id + 1) as u64), executable_hash: Some(hash), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/eventbus_performance_comparison.rs b/collector-core/tests/eventbus_performance_comparison.rs index 0248c918..827909ba 100644 --- a/collector-core/tests/eventbus_performance_comparison.rs +++ b/collector-core/tests/eventbus_performance_comparison.rs @@ -161,6 +161,7 @@ fn create_test_events(count: usize) -> Vec { cpu_usage: Some(1.0 + (i as f64 % 5.0)), memory_usage: Some(1024 * 1024 + (i as u64 * 1024)), executable_hash: Some(format!("hash_{:08x}", i)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/integration_test.rs b/collector-core/tests/integration_test.rs index b12e0928..59ac1889 100644 --- a/collector-core/tests/integration_test.rs +++ b/collector-core/tests/integration_test.rs @@ -119,6 +119,7 @@ impl EventSource for MockProcessSource { cpu_usage: Some(1.5), memory_usage: Some(1024 * 1024), executable_hash: Some("mock_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -269,6 +270,7 @@ async fn test_event_types() { cpu_usage: Some(5.5), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/multi_collector_coordination.rs b/collector-core/tests/multi_collector_coordination.rs index 45de7234..90ccc7cd 100644 --- a/collector-core/tests/multi_collector_coordination.rs +++ b/collector-core/tests/multi_collector_coordination.rs @@ -101,6 +101,7 @@ async fn test_multi_collector_task_distribution() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -201,6 +202,7 @@ async fn test_result_aggregation() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -363,6 +365,7 @@ async fn test_complete_coordination_workflow() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/tests/performance_critical_test.rs b/collector-core/tests/performance_critical_test.rs index ebc90d7b..cb85d315 100644 --- a/collector-core/tests/performance_critical_test.rs +++ b/collector-core/tests/performance_critical_test.rs @@ -114,6 +114,7 @@ impl EventSource for PerformanceTestSource { cpu_usage: Some(0.1), memory_usage: Some(1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/performance_tests.rs b/collector-core/tests/performance_tests.rs index 19266731..68ccb0d2 100644 --- a/collector-core/tests/performance_tests.rs +++ b/collector-core/tests/performance_tests.rs @@ -95,6 +95,7 @@ async fn test_high_process_churn_monitoring() { cpu_usage: Some(1.0 + (i as f64 % 5.0)), memory_usage: Some(1024 * 1024 + (i as u64 * 1024)), executable_hash: Some(format!("hash_{}", i)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -298,6 +299,7 @@ async fn test_baseline_establishment() { cpu_usage: Some(2.0), memory_usage: Some(2 * 1024 * 1024), executable_hash: Some("baseline_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -415,6 +417,7 @@ async fn test_concurrent_monitoring_operations() { cpu_usage: Some(1.5), memory_usage: Some(1024 * 1024), executable_hash: Some("concurrent_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -604,6 +607,7 @@ async fn test_disabled_monitoring_performance() { cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("disabled_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/property_based_test.rs b/collector-core/tests/property_based_test.rs index a7923048..d4861a9d 100644 --- a/collector-core/tests/property_based_test.rs +++ b/collector-core/tests/property_based_test.rs @@ -156,6 +156,7 @@ fn process_event_strategy() -> impl Strategy { cpu_usage, memory_usage, executable_hash, + hash_algorithm: None, user_id, accessible, file_exists, @@ -634,6 +635,7 @@ mod deterministic_tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/tests/security_critical_test.rs b/collector-core/tests/security_critical_test.rs index 600f3e5c..5298e6be 100644 --- a/collector-core/tests/security_critical_test.rs +++ b/collector-core/tests/security_critical_test.rs @@ -115,6 +115,7 @@ impl EventSource for SecurityTestSource { cpu_usage: Some(0.05), memory_usage: Some(512), executable_hash: Some("secure123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/security_isolation_test.rs b/collector-core/tests/security_isolation_test.rs index ac9dd195..15b7c954 100644 --- a/collector-core/tests/security_isolation_test.rs +++ b/collector-core/tests/security_isolation_test.rs @@ -182,6 +182,7 @@ impl EventSource for SecurityTestSource { cpu_usage: Some(0.1), memory_usage: Some(1024), executable_hash: Some("security_test_hash".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/shared_infrastructure_test.rs b/collector-core/tests/shared_infrastructure_test.rs index de2f42d3..24f29caf 100644 --- a/collector-core/tests/shared_infrastructure_test.rs +++ b/collector-core/tests/shared_infrastructure_test.rs @@ -130,6 +130,7 @@ impl EventSource for TestEventSource { cpu_usage: Some(1.0 + i as f64), memory_usage: Some(1024 * (i + 1) as u64), executable_hash: Some(format!("hash_{}", i)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/collector-core/tests/simple_daemoneye_test.rs b/collector-core/tests/simple_daemoneye_test.rs index 9f5bb579..8dbe783e 100644 --- a/collector-core/tests/simple_daemoneye_test.rs +++ b/collector-core/tests/simple_daemoneye_test.rs @@ -163,6 +163,7 @@ async fn test_daemoneye_eventbus_publish_only() { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/collector-core/tests/test_stack_overflow_debug.rs b/collector-core/tests/test_stack_overflow_debug.rs index cab79f73..fa984040 100644 --- a/collector-core/tests/test_stack_overflow_debug.rs +++ b/collector-core/tests/test_stack_overflow_debug.rs @@ -145,6 +145,7 @@ impl EventSource for SimpleTestSource { cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some(format!("hash_{}", i)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/daemoneye-lib/Cargo.toml b/daemoneye-lib/Cargo.toml index b069e050..29a653f0 100644 --- a/daemoneye-lib/Cargo.toml +++ b/daemoneye-lib/Cargo.toml @@ -34,6 +34,18 @@ network-correlation = [] # IPC transport features ipc = [] # Use interprocess transport (cross-platform) +# Integrity / hashing features +# +# Per AGENTS.md Cryptographic Standards: SHA-1 is forbidden. We deliberately +# do NOT expose a `legacy-hashes` feature for SHA-1 or MD5, even gated, so +# that broken primitives cannot be pulled into the build graph without an +# explicit code change and security review. Any future need for weak hashes +# (e.g. correlation with third-party data sources that only publish MD5) +# must land as a separate, documented exception. +# +# sha3-hashes: SHA3-256 (cryptographically secure, ~5-10x slower than SHA-256) +sha3-hashes = ["dep:sha3"] + [dependencies] anyhow = { workspace = true } @@ -75,6 +87,12 @@ serde = { workspace = true } serde_json = { workspace = true } sha2 = { workspace = true } +# Optional: additional cryptographic hash backends for integrity verification +sha3 = { workspace = true, optional = true } + +# Shared bounded hash-result cache (inline path + triggered path share one instance) +quick_cache = { workspace = true } + # SQL parsing and validation sqlparser = { workspace = true, optional = true } @@ -139,3 +157,7 @@ harness = false [[bench]] name = "ipc_client_validation_benchmarks" harness = false + +[[bench]] +name = "integrity_operations" +harness = false diff --git a/daemoneye-lib/benches/integrity_operations.rs b/daemoneye-lib/benches/integrity_operations.rs new file mode 100644 index 00000000..47b19b5b --- /dev/null +++ b/daemoneye-lib/benches/integrity_operations.rs @@ -0,0 +1,131 @@ +// Benchmarks intentionally use `.expect(...)` for setup failures: a +// benchmark harness that silently eats errors is worse than a loud panic, +// and there is no user-facing error path to preserve. Scoped crate-wide +// because nearly every function performs setup. Other clippy lints +// (uninlined_format_args, shadow_reuse, as_conversions, indexing_slicing) +// are NOT allowed here — if a future edit trips one, fix the code instead +// of silencing the warning. +#![allow(clippy::expect_used)] + +//! Criterion benchmarks for the `daemoneye_lib::integrity` module. +//! +//! Measures streaming hash throughput across representative file sizes and +//! algorithm selections through the public [`MultiAlgorithmHasher`] API. +//! The goal is to guard the hot paths used by both procmond's inline +//! enumeration hashing and the triggered `BinaryHasherCollector`: +//! +//! - 1 KiB warm: <50 µs (guards the `spawn_blocking` threshold) +//! - 256 KiB warm, BLAKE3+SHA-256: <5 ms +//! - 4 MiB warm, BLAKE3+SHA-256: <100 ms +//! +//! Each benchmark hashes a real on-disk file through the engine's +//! `HashComputer` trait so the measurement path matches production — +//! including file open, metadata fstat, and cache lookup. + +use criterion::{Criterion, criterion_group, criterion_main}; +use daemoneye_lib::integrity::{HashAlgorithm, HashComputer, HasherConfig, MultiAlgorithmHasher}; +use std::hint::black_box; +use std::io::Write; +use tempfile::NamedTempFile; +use tokio::runtime::Runtime; + +/// Create a temp file filled with `size` bytes of deterministic data. +fn make_file(size: usize) -> NamedTempFile { + let mut tmp = NamedTempFile::new().expect("create temp file"); + // Deterministic non-zero payload so compiler/hardware hashers cannot + // short-circuit a zero-page fast path. + let chunk: Vec = (0..=255_u8).cycle().take(size).collect(); + tmp.write_all(&chunk).expect("write temp file"); + tmp.flush().expect("flush temp file"); + tmp +} + +/// Build a hasher with the requested algorithm list and a disabled cache so +/// each iteration hashes the file end-to-end. +fn build_hasher(algorithms: Vec) -> MultiAlgorithmHasher { + let config = HasherConfig::default() + .with_algorithms(algorithms) + .with_cache_capacity(0); + MultiAlgorithmHasher::new(config).expect("hasher config validates") +} + +fn bench_hash_sha256_only_small(c: &mut Criterion) { + let rt = Runtime::new().expect("tokio runtime"); + let tmp = make_file(1024); + let hasher = build_hasher(vec![HashAlgorithm::Sha256]); + c.bench_function("integrity_hash_sha256_only_1kib", |b| { + b.iter(|| { + rt.block_on(async { + black_box( + hasher + .compute(tmp.path()) + .await + .expect("hash small sha256-only"), + ) + }); + }); + }); +} + +fn bench_hash_multi_algo_small(c: &mut Criterion) { + let rt = Runtime::new().expect("tokio runtime"); + let tmp = make_file(1024); + let hasher = build_hasher(vec![HashAlgorithm::Sha256, HashAlgorithm::Blake3]); + c.bench_function("integrity_hash_multi_algo_1kib", |b| { + b.iter(|| { + rt.block_on(async { + black_box( + hasher + .compute(tmp.path()) + .await + .expect("hash small multi-algo"), + ) + }); + }); + }); +} + +fn bench_hash_multi_algo_medium(c: &mut Criterion) { + let rt = Runtime::new().expect("tokio runtime"); + let tmp = make_file(256 * 1024); + let hasher = build_hasher(vec![HashAlgorithm::Sha256, HashAlgorithm::Blake3]); + c.bench_function("integrity_hash_multi_algo_256kib", |b| { + b.iter(|| { + rt.block_on(async { + black_box( + hasher + .compute(tmp.path()) + .await + .expect("hash medium multi-algo"), + ) + }); + }); + }); +} + +fn bench_hash_multi_algo_large(c: &mut Criterion) { + let rt = Runtime::new().expect("tokio runtime"); + let tmp = make_file(4 * 1024 * 1024); + let hasher = build_hasher(vec![HashAlgorithm::Sha256, HashAlgorithm::Blake3]); + c.bench_function("integrity_hash_multi_algo_4mib", |b| { + b.iter(|| { + rt.block_on(async { + black_box( + hasher + .compute(tmp.path()) + .await + .expect("hash large multi-algo"), + ) + }); + }); + }); +} + +criterion_group!( + benches, + bench_hash_sha256_only_small, + bench_hash_multi_algo_small, + bench_hash_multi_algo_medium, + bench_hash_multi_algo_large +); +criterion_main!(benches); diff --git a/daemoneye-lib/src/integrity/auth.rs b/daemoneye-lib/src/integrity/auth.rs new file mode 100644 index 00000000..81ceb148 --- /dev/null +++ b/daemoneye-lib/src/integrity/auth.rs @@ -0,0 +1,353 @@ +//! Shared authorization predicates for executable hashing. +//! +//! These helpers are consumed by both: +//! - `procmond/src/hash_pass.rs` (`authorize_kernel_path`) for the +//! post-enumeration path where sysinfo supplies kernel-resolved exe paths. +//! - `collector-core/src/binary_hasher.rs` (`authorize_confined_path`) for +//! the triggered path where cap-std `Dir` handles confine opens. +//! +//! All predicates operate on **byte lengths only** — they never index or +//! slice into the path string (CWE-135 safety). + +use std::path::Path; +use thiserror::Error; + +/// Linux `PATH_MAX`. Used as a sanity bound on executable paths. +/// +/// This is a byte-length comparison only — no slicing, no indexing. +/// The prior value of 107 (Unix `sockaddr_un.sun_path`) was +/// architecturally incorrect: it conflates socket path limits with +/// filesystem path limits. +pub const MAX_EXECUTABLE_PATH_LEN: usize = 4096; + +/// Maximum file size (bytes) for the authorization pre-open gate. +/// +/// Distinct from the engine's `HasherConfig::max_file_size` — this rejects +/// absurdly large binaries before the engine ever opens them. +/// Defaults to 512 MiB (matches the engine default). +pub const MAX_EXECUTABLE_FILE_SIZE: u64 = 512 * 1024 * 1024; + +/// Errors from the authorization predicates. +/// +/// These are distinct from [`super::HashError`] because authorization +/// happens *before* the engine opens the file. A path rejected here +/// never reaches the hash engine. +#[derive(Debug, Error)] +#[non_exhaustive] +pub enum AuthError { + /// Path exceeds [`MAX_EXECUTABLE_PATH_LEN`] bytes. + #[error("path too long: {len} bytes exceeds limit of {limit} bytes")] + PathTooLong { + /// Observed byte length. + len: usize, + /// Configured limit. + limit: usize, + }, + + /// Path is not a regular file (symlink, directory, device, etc.). + #[error("not a regular file: {}", path.display())] + NotRegularFile { + /// The path that failed the check. + path: std::path::PathBuf, + }, + + /// File exceeds the authorization-layer size limit. + #[error("file too large for hashing: {size} bytes exceeds {limit} bytes")] + FileTooLarge { + /// Observed file size. + size: u64, + /// Configured limit. + limit: u64, + }, + + /// Path contains traversal components (`..`). + #[error("path contains traversal component: {}", path.display())] + PathTraversal { + /// The path that failed the check. + path: std::path::PathBuf, + }, + + /// Underlying I/O error during metadata checks. + #[error("I/O error checking {}: {source}", path.display())] + Io { + /// The path being checked. + path: std::path::PathBuf, + /// Underlying error. + #[source] + source: std::io::Error, + }, + + /// cap-std rejected the open because the path escaped the `Dir` root. + /// + /// This fires when a symlink, bind-mount, or `..` traversal attempts + /// to reach outside the allowed root. + #[error("path escaped allowed root: {message}")] + CapStdEscape { + /// The cap-std error message (preserved for regression testing). + message: String, + }, + + /// Target is a symbolic link and the policy forbids following. + #[error("target is a symlink: {}", path.display())] + SymlinkRejected { + /// The path that is a symlink. + path: std::path::PathBuf, + }, + + /// Path is outside all configured allowed roots. + #[error("path not under any allowed root: {}", path.display())] + PathNotAllowed { + /// The path that failed the check. + path: std::path::PathBuf, + }, + + /// macOS: the allowed root's (`st_dev`, `st_ino`) fingerprint changed, + /// indicating a bind-mount or volume swap. + #[cfg(target_os = "macos")] + #[error("root mount point changed: {root}")] + RootMountChanged { + /// The root whose fingerprint no longer matches. + root: String, + }, +} + +/// Check that `path`'s byte length does not exceed `MAX_EXECUTABLE_PATH_LEN`. +/// +/// Uses `as_os_str().len()` which returns the byte length on Unix and the +/// WTF-8 byte length on Windows — never indexes or slices the string. +/// +/// # Errors +/// +/// Returns [`AuthError::PathTooLong`] if the path exceeds the limit. +pub fn check_path_length(path: &Path) -> Result<(), AuthError> { + let len = path.as_os_str().len(); + if len > MAX_EXECUTABLE_PATH_LEN { + return Err(AuthError::PathTooLong { + len, + limit: MAX_EXECUTABLE_PATH_LEN, + }); + } + Ok(()) +} + +/// Check that `path` contains no `..` traversal components. +/// +/// # Errors +/// +/// Returns [`AuthError::PathTraversal`] if any component is `..`. +pub fn check_no_traversal(path: &Path) -> Result<(), AuthError> { + for component in path.components() { + if matches!(component, std::path::Component::ParentDir) { + return Err(AuthError::PathTraversal { + path: path.to_path_buf(), + }); + } + } + Ok(()) +} + +/// Check that `metadata` describes a regular file (and not a symlink). +/// +/// Also rejects symlinks explicitly via `file_type().is_symlink()` so the +/// no-symlink policy holds even if a caller accidentally passes +/// [`std::fs::metadata`] (which follows symlinks) instead of +/// [`std::fs::symlink_metadata`]. When the metadata comes from +/// `symlink_metadata`, a symlink has `file_type().is_symlink() == true` +/// and we return [`AuthError::SymlinkRejected`]. When it comes from +/// `metadata`, the target's type is reported and a non-file target still +/// trips the `!is_file()` branch below. +/// +/// # Safety contract for callers +/// +/// Callers that want to reject symlinks (the default no-follow policy in +/// this workspace) **must** obtain `metadata` via +/// [`std::fs::symlink_metadata`]. Passing regular `std::fs::metadata` +/// would silently follow the symlink, so the target is checked instead +/// of the link itself — this function cannot fully compensate for that +/// at the type level because it receives metadata, not a path. +/// +/// # Errors +/// +/// Returns [`AuthError::SymlinkRejected`] if the metadata is for a +/// symbolic link, or [`AuthError::NotRegularFile`] if it is not a +/// regular file for any other reason. +pub fn check_regular_file(path: &Path, metadata: &std::fs::Metadata) -> Result<(), AuthError> { + if metadata.file_type().is_symlink() { + return Err(AuthError::SymlinkRejected { + path: path.to_path_buf(), + }); + } + if !metadata.is_file() { + return Err(AuthError::NotRegularFile { + path: path.to_path_buf(), + }); + } + Ok(()) +} + +/// Check that the file size does not exceed `limit`. +/// +/// # Errors +/// +/// Returns [`AuthError::FileTooLarge`] if `metadata.len() > limit`. +pub fn check_size(metadata: &std::fs::Metadata, limit: u64) -> Result<(), AuthError> { + let size = metadata.len(); + if size > limit { + return Err(AuthError::FileTooLarge { size, limit }); + } + Ok(()) +} + +/// Truncate a path to at most `max_bytes` for safe logging. +/// +/// Uses a per-`char` UTF-8-aware accumulator so the truncation point always +/// falls on a valid code-point boundary — never producing a partial +/// multi-byte sequence (CWE-135). Non-UTF-8 paths are lossily converted +/// first. The returned string is guaranteed to be at most `max_bytes` bytes +/// long, including the trailing `"..."` marker. When `max_bytes <= 3` and +/// the path overflows the budget, the result is at most `max_bytes` +/// characters of the ellipsis (never slicing mid-char). +#[must_use] +pub fn bytes_safe_display(path: &Path, max_bytes: usize) -> String { + let lossy = path.to_string_lossy(); + let byte_len = lossy.len(); + if byte_len <= max_bytes { + return lossy.into_owned(); + } + // Tiny budgets: return at most max_bytes bytes of ellipsis, never + // slicing mid-char. Each '.' is one byte so this is byte-safe. + if max_bytes <= 3 { + return "...".chars().take(max_bytes).collect(); + } + // Reserve 3 bytes for the trailing ellipsis, then collect code points + // whose UTF-8 length fits in the remaining budget. + let budget = max_bytes.saturating_sub(3); + let mut truncated = String::with_capacity(max_bytes); + let mut used: usize = 0; + for ch in lossy.chars() { + let ch_len = ch.len_utf8(); + if used.saturating_add(ch_len) > budget { + break; + } + truncated.push(ch); + used = used.saturating_add(ch_len); + } + truncated.push_str("..."); + truncated +} + +#[cfg(test)] +#[allow(clippy::expect_used, clippy::unwrap_used)] +mod tests { + use super::*; + use std::path::PathBuf; + + #[test] + fn path_length_at_boundary() { + // Exactly 4096 bytes — should pass. + let path = PathBuf::from("a".repeat(MAX_EXECUTABLE_PATH_LEN)); + assert!(check_path_length(&path).is_ok()); + } + + #[test] + fn path_length_one_over() { + let path = PathBuf::from("a".repeat(MAX_EXECUTABLE_PATH_LEN + 1)); + assert!(matches!( + check_path_length(&path), + Err(AuthError::PathTooLong { .. }) + )); + } + + #[test] + fn path_length_with_emoji_no_panic() { + // 4096 emoji characters = 16384 bytes. Must not panic (CWE-135). + let emoji_path = PathBuf::from("\u{1F600}".repeat(MAX_EXECUTABLE_PATH_LEN)); + let result = check_path_length(&emoji_path); + assert!(matches!(result, Err(AuthError::PathTooLong { .. }))); + } + + #[test] + fn traversal_detected() { + let path = PathBuf::from("/usr/bin/../sbin/evil"); + assert!(matches!( + check_no_traversal(&path), + Err(AuthError::PathTraversal { .. }) + )); + } + + #[test] + fn clean_path_passes_traversal() { + let path = PathBuf::from("/usr/bin/ls"); + assert!(check_no_traversal(&path).is_ok()); + } + + #[test] + fn bytes_safe_display_truncates_on_char_boundary() { + // 12-byte emoji path with budget 7: reserve 3 bytes for "...", + // leaving 4 bytes -> fits exactly one emoji (4 bytes) + "...". + let path = PathBuf::from("\u{1F600}\u{1F600}\u{1F600}"); // 12 bytes + let display = bytes_safe_display(&path, 7); + assert_eq!(display, "\u{1F600}..."); + assert!(display.len() <= 7); + } + + #[test] + fn bytes_safe_display_respects_max_bytes_budget() { + // Exhaustively check that output never exceeds max_bytes for a + // long multi-byte path across a range of budgets. + let path = PathBuf::from("\u{1F600}".repeat(50)); // 200 bytes + for max_bytes in 0..=64 { + let display = bytes_safe_display(&path, max_bytes); + assert!( + display.len() <= max_bytes, + "result `{display}` exceeded budget {max_bytes}", + ); + } + } + + #[test] + fn bytes_safe_display_short_path_unchanged() { + let path = PathBuf::from("/bin/ls"); + let display = bytes_safe_display(&path, 100); + assert_eq!(display, "/bin/ls"); + } + + #[test] + fn bytes_safe_display_tiny_budget_no_panic() { + // max_bytes <= 3 must return at most max_bytes characters of "..." + // without panicking or slicing mid-char. + let path = PathBuf::from("/very/long/path/for/logging"); + assert_eq!(bytes_safe_display(&path, 0), ""); + assert_eq!(bytes_safe_display(&path, 1), "."); + assert_eq!(bytes_safe_display(&path, 2), ".."); + assert_eq!(bytes_safe_display(&path, 3), "..."); + } + + #[test] + fn bytes_safe_display_budget_four_is_ellipsis_only() { + // 4-byte budget reserves 3 for "...", leaving 1 byte. A 4-byte + // emoji cannot fit, so output is just "...". + let path = PathBuf::from("\u{1F600}\u{1F600}"); + let display = bytes_safe_display(&path, 4); + assert_eq!(display, "..."); + assert!(display.len() <= 4); + } + + #[test] + fn check_size_passes_under_limit() { + let tmp = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(tmp.path(), b"small").unwrap(); + let meta = std::fs::metadata(tmp.path()).unwrap(); + assert!(check_size(&meta, MAX_EXECUTABLE_FILE_SIZE).is_ok()); + } + + #[test] + fn check_regular_file_rejects_dir() { + let dir = tempfile::tempdir().unwrap(); + let meta = std::fs::metadata(dir.path()).unwrap(); + assert!(matches!( + check_regular_file(dir.path(), &meta), + Err(AuthError::NotRegularFile { .. }) + )); + } +} diff --git a/daemoneye-lib/src/integrity/mod.rs b/daemoneye-lib/src/integrity/mod.rs new file mode 100644 index 00000000..672280a3 --- /dev/null +++ b/daemoneye-lib/src/integrity/mod.rs @@ -0,0 +1,1478 @@ +//! Cryptographic integrity verification (binary hashing). +//! +//! Streaming multi-algorithm hash engine for executables and critical system +//! files. Produces a `HashResult` with cryptographically secure hashes +//! (SHA-256, BLAKE3 by default; SHA-3-256 behind `sha3-hashes`). +//! +//! # Design +//! +//! - **Streaming reads**: 256 KiB buffer, bounded memory regardless of file size. +//! - **Cooperative cancellation**: deadlines enforced via an `AtomicBool` flag +//! checked inside the read loop. `tokio::time::timeout` alone cannot cancel +//! `spawn_blocking` tasks — the blocking thread would keep running and hold +//! its semaphore permit until the loop completes naturally. See +//! . +//! - **Size-threshold dispatch**: files smaller than `SPAWN_BLOCKING_THRESHOLD` +//! are hashed inline on the current task (pure CPU, ~microseconds). Larger +//! files go through `tokio::task::spawn_blocking`. This mirrors how +//! `tokio::fs` makes the same tradeoff internally. +//! - **Bounded concurrency**: an `Arc` sized from +//! `std::thread::available_parallelism()` (clamped to `[2, 16]`) caps +//! simultaneous hash operations across all callers. Both the inline +//! enumeration path and the on-demand triggered path share a single engine +//! instance so the combined load always respects the cap. +//! - **TOCTOU tagging at the engine boundary**: `(size, mtime)` are captured +//! before and after the read. If they drift, the engine returns +//! `HashError::Nonauthoritative` — a mid-read mutation is forensic +//! evidence, not a successful hash. Callers that need detection signal +//! should observe the `Nonauthoritative` error path specifically; callers +//! that just want a usable hash get type-state safety because +//! `HashResult` is unreachable in the error case. +//! - **Shared cache**: a `quick_cache::sync::Cache` keyed by +//! `(PathBuf, SystemTime, u64)` is optionally held by the engine so that +//! both the inline enumeration path (procmond) and the on-demand triggered +//! path (`BinaryHasherCollector`) observe a consistent view of a file +//! snapshot. Cross-path cache hits are expected to be rare in steady state; +//! the primary value of sharing the engine is **single policy** (one +//! concurrency cap, one algorithm list, one size limit), not throughput. +//! +//! # Algorithm policy +//! +//! - **Default**: SHA-256 + BLAKE3 (both cryptographically secure). +//! - **Opt-in via feature flags**: SHA-3-256 (`sha3-hashes`). +//! - **Deliberately unsupported**: SHA-1 and MD5. Per `DaemonEye`'s +//! cryptographic standards (see `AGENTS.md` — "never SHA-1"), weak hashes +//! are not compiled into the binary under any feature flag. Downstream +//! code must still use `HashAlgorithm::is_cryptographically_secure` to +//! gate trust decisions, so the API remains forward-compatible if a +//! future secure algorithm is ever added. +//! +//! # Example +//! +//! ```no_run +//! use daemoneye_lib::integrity::{ +//! HashAlgorithm, HashComputer, HasherConfig, MultiAlgorithmHasher, +//! }; +//! use std::path::Path; +//! +//! # async fn run() -> Result<(), Box> { +//! let hasher = MultiAlgorithmHasher::new(HasherConfig::default())?; +//! let result = hasher.compute(Path::new("/bin/ls")).await?; +//! let sha256_hex = result.hashes.get(&HashAlgorithm::Sha256).ok_or("no sha256")?; +//! println!("sha256 = {sha256_hex}"); +//! # Ok(()) +//! # } +//! ``` + +pub mod auth; + +use quick_cache::sync::Cache; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::fmt; +use std::io::{self, Read}; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::time::{Duration, Instant, SystemTime}; +use thiserror::Error; +use tokio::sync::Semaphore; +use tracing::{debug, warn}; + +use sha2::{Digest as Sha2Digest, Sha256}; + +#[cfg(feature = "sha3-hashes")] +use sha3::Sha3_256; + +// ───────────────────────────────────────────────────────────────────────────── +// Constants +// ───────────────────────────────────────────────────────────────────────────── + +/// Streaming read buffer size (256 KiB). +/// +/// Aligns with Linux readahead defaults, macOS APFS I/O units, and Windows +/// NTFS sequential-scan behavior. Large enough to keep BLAKE3 SIMD paths +/// saturated; small enough that 16 concurrent operations stay well under +/// the 100 MiB workspace memory budget. +pub const BUFFER_SIZE: usize = 256 * 1024; + +/// Size threshold below which files are hashed inline. +/// +/// Files smaller than this are hashed on the current async task (pure CPU, +/// ~microseconds, no blocking-pool round-trip). Files at or above are routed +/// through `tokio::task::spawn_blocking`. Matches the heuristic that +/// `tokio::fs` uses internally for its own I/O dispatch decisions. +pub const SPAWN_BLOCKING_THRESHOLD: u64 = 256 * 1024; + +/// Default maximum file size accepted for hashing (512 MiB). Larger files are +/// rejected with [`HashError::FileTooLarge`] to bound both memory and time. +pub const DEFAULT_MAX_FILE_SIZE: u64 = 512 * 1024 * 1024; + +/// Default per-file hash deadline (10 seconds). +pub const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); + +/// Default bounded-cache capacity in entries. +pub const DEFAULT_CACHE_CAPACITY: usize = 10_000; + +/// Lower bound on `max_concurrent`. Even on 1-vCPU hosts we allow 2 concurrent +/// operations so the triggered path isn't starved by the inline path. +pub const MIN_CONCURRENCY: usize = 2; + +/// Upper bound on `max_concurrent`. I/O queue depth rarely benefits from more +/// than ~16 parallel hash operations on commodity storage. +pub const MAX_CONCURRENCY: usize = 16; + +// ───────────────────────────────────────────────────────────────────────────── +// HashAlgorithm +// ───────────────────────────────────────────────────────────────────────────── + +/// Supported cryptographic hash algorithms. +/// +/// **ORDER IS LOAD-BEARING**: `HashAlgorithm: Ord` derives its ordering from +/// the discriminant order, and [`HashResult::hashes`] is a `BTreeMap` whose +/// serialization depends on that ordering. Snapshot fixtures and the audit +/// ledger's BLAKE3 hash chain indirectly depend on stable serialization. +/// **Never reorder variants and never change an existing discriminant +/// value.** Appending new variants at the end with a fresh discriminant is +/// fine. Explicit discriminants make the ordering immutable even if someone +/// accidentally reshuffles the source lines. +#[repr(u8)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] +#[non_exhaustive] +#[serde(rename_all = "kebab-case")] +pub enum HashAlgorithm { + /// SHA-256 — NIST FIPS 180-4. Cryptographically secure. Primary integrity + /// hash; the default value stored in + /// [`crate::models::process::ProcessRecord::executable_hash`]. + Sha256 = 0, + /// BLAKE3 — modern, very fast, cryptographically secure. Enabled by + /// default; already a workspace dependency via the audit ledger chain. + Blake3 = 1, + /// SHA3-256 — NIST FIPS 202. Cryptographically secure. 5–10× slower than + /// SHA-256 on CPUs with SHA-NI. Behind the `sha3-hashes` feature. + #[cfg(feature = "sha3-hashes")] + Sha3_256 = 2, +} + +impl HashAlgorithm { + /// Whether this algorithm may be used to make trust or integrity + /// decisions. All currently-supported variants return `true`; the method + /// is kept so downstream code gates trust behind an explicit check and + /// so the return value can flip to `false` without an API break if a + /// weak algorithm is ever (re-)introduced. + #[must_use] + pub const fn is_cryptographically_secure(self) -> bool { + #[cfg_attr(not(feature = "sha3-hashes"), allow(clippy::match_same_arms))] + match self { + Self::Sha256 => true, + Self::Blake3 => true, + #[cfg(feature = "sha3-hashes")] + Self::Sha3_256 => true, + } + } + + /// Canonical lowercase wire name used in the protobuf `hash_algorithm` + /// field at `daemoneye-lib/proto/common.proto`. + #[must_use] + pub const fn wire_name(self) -> &'static str { + match self { + Self::Sha256 => "sha256", + Self::Blake3 => "blake3", + #[cfg(feature = "sha3-hashes")] + Self::Sha3_256 => "sha3-256", + } + } + + /// Output length in hex characters. Useful for test vector assertions. + #[must_use] + pub const fn hex_len(self) -> usize { + match self { + Self::Sha256 => 64, + Self::Blake3 => 64, + #[cfg(feature = "sha3-hashes")] + Self::Sha3_256 => 64, + } + } +} + +impl fmt::Display for HashAlgorithm { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.wire_name()) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// HashIntegrity +// ───────────────────────────────────────────────────────────────────────────── + +/// Integrity tag on a successful [`HashResult`]. +/// +/// # Single-variant design (intentional) +/// +/// After the Nonauthoritative type-state refactor (2026-04-09) this enum +/// intentionally has exactly one variant. The former `FileChanged` variant has +/// been removed and its semantics moved to [`HashError::Nonauthoritative`]: +/// mid-read mutations are now surfaced as an `Err` at the engine boundary +/// rather than flowing through the `Ok` path as a degraded integrity tag. +/// This means every `Ok(HashResult)` is [`HashIntegrity::Stable`] by +/// construction — callers no longer need to inspect the tag to decide whether +/// bytes are trustworthy. +/// +/// # Forward-compatibility extension point +/// +/// The enum is deliberately kept (rather than replaced by a unit struct or +/// removed entirely) to preserve room for future non-failure integrity +/// annotations — for example, a `SnapshotVerified` or `MerkleProofed` tag — +/// without requiring a breaking API change. `#[non_exhaustive]` reinforces +/// this intent: downstream `match` arms **must** include a wildcard arm so +/// that adding new variants here never breaks consumers. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[non_exhaustive] +#[serde(rename_all = "snake_case")] +pub enum HashIntegrity { + /// File metadata was consistent before and after the hash read. + Stable, +} + +// ───────────────────────────────────────────────────────────────────────────── +// HashResult +// ───────────────────────────────────────────────────────────────────────────── + +/// Result of a hashing operation. Immutable value type. +/// +/// Only cryptographically secure hashes are ever populated in [`Self::hashes`]. +/// SHA-1 and MD5 are deliberately not supported by the engine, so the hash +/// map can be trusted at the type level to hold only algorithms approved by +/// `DaemonEye`'s cryptographic standards. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct HashResult { + /// File path the hash was computed against. This is the path that + /// was passed to `compute()` or `compute_from_file()` — it is NOT + /// canonicalized by the engine. Callers that want a canonical path + /// in the audit record must pass one themselves; the cap-std-based + /// `BinaryHasherCollector` already does this when `follow_symlinks` + /// is set. + pub file_path: PathBuf, + /// File size in bytes as observed at open time. + pub file_size: u64, + /// File modification time as observed at open time. + pub modified_time: SystemTime, + /// Cryptographically secure hashes, hex-encoded. Safe for lifecycle + /// diffing, detection rule evaluation, and audit ledger inclusion. + pub hashes: BTreeMap, + /// Integrity tag. Always [`HashIntegrity::Stable`] in the `Ok` path — + /// mid-read mutations are returned as [`HashError::Nonauthoritative`] + /// at the engine boundary, so a successful `HashResult` cannot carry + /// non-authoritative bytes. + pub integrity: HashIntegrity, + /// Wall-clock time spent computing the hash. + pub computation_time: Duration, +} + +impl HashResult { + /// Returns the SHA-256 hex string if present. Convenience for the + /// inline enumeration path, which stores only SHA-256 in the protobuf + /// `executable_hash` field. + #[must_use] + pub fn sha256(&self) -> Option<&str> { + self.hashes.get(&HashAlgorithm::Sha256).map(String::as_str) + } + + /// Returns the BLAKE3 hex string if present. + #[must_use] + pub fn blake3(&self) -> Option<&str> { + self.hashes.get(&HashAlgorithm::Blake3).map(String::as_str) + } + + /// Whether the result is safe to use for integrity or lifecycle diff + /// decisions. Returns `false` when the file mutated during the read. + #[must_use] + pub const fn is_authoritative(&self) -> bool { + matches!(self.integrity, HashIntegrity::Stable) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// HashError +// ───────────────────────────────────────────────────────────────────────────── + +/// Errors returned by [`HashComputer::compute`] and related methods. +#[derive(Debug, Error)] +#[non_exhaustive] +pub enum HashError { + /// The filesystem refused to open or read the file. + #[error("permission denied: {path}")] + PermissionDenied { + /// Path that could not be opened. + path: PathBuf, + }, + /// The file did not exist at open time. + #[error("file not found: {path}")] + FileNotFound { + /// Path that could not be opened. + path: PathBuf, + }, + /// The file exceeded the configured maximum size. + #[error("file too large: {size} bytes exceeds limit of {limit} bytes")] + FileTooLarge { + /// Observed file size. + size: u64, + /// Configured limit. + limit: u64, + }, + /// The hash operation exceeded its deadline. Cooperative cancellation + /// detected the timeout on its next read-loop iteration. + #[error("hash operation timed out")] + Timeout, + /// The hash operation was cancelled by the caller. + #[error("hash operation cancelled")] + Cancelled, + /// A lower-level I/O error occurred during read. + #[error("I/O error hashing {path}: {source}")] + Io { + /// Path being read when the error occurred. + path: PathBuf, + /// Underlying I/O error. + #[source] + source: io::Error, + }, + /// `spawn_blocking` task panicked or was cancelled externally. + #[error("blocking task join failed: {0}")] + Join(String), + /// Caller supplied a configuration that could not be satisfied. + #[error("invalid hasher configuration: {0}")] + InvalidConfig(String), + /// The file was mutated between the open-time and close-time metadata + /// snapshots. The hashed bytes are a mid-read sample and are + /// **not authoritative** — they must never be trusted for integrity + /// decisions or stamped into a lifecycle diff. Receiving this error is + /// a detection signal: a binary was modified while being hashed. + /// + /// This variant is returned by [`HashComputer::compute`] at the engine + /// boundary (not by a display-layer accessor) so type-state guarantees + /// that a successful `Ok(HashResult)` always carries stable bytes. + #[error("file mutated during hash: {path}")] + Nonauthoritative { + /// The path that was being hashed when the mid-read mutation was + /// detected. + path: PathBuf, + }, +} + +impl HashError { + /// Maps an `io::Error` at a specific path into a structured `HashError`. + /// + /// Only `NotFound` and `PermissionDenied` get dedicated variants — every + /// other `ErrorKind` is bundled into [`HashError::Io`]. The wildcard arm + /// is deliberate: `io::ErrorKind` is `#[non_exhaustive]` upstream and + /// holds dozens of variants we do not want to treat individually. The + /// upstream `#[non_exhaustive]` attribute also means future kinds will + /// flow through this arm automatically without a silent regression. + #[allow( + clippy::wildcard_enum_match_arm, + reason = "io::ErrorKind is #[non_exhaustive] and has many irrelevant variants" + )] + fn from_io(path: &Path, err: io::Error) -> Self { + match err.kind() { + io::ErrorKind::NotFound => Self::FileNotFound { + path: path.to_path_buf(), + }, + io::ErrorKind::PermissionDenied => Self::PermissionDenied { + path: path.to_path_buf(), + }, + _ => Self::Io { + path: path.to_path_buf(), + source: err, + }, + } + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// HasherConfig +// ───────────────────────────────────────────────────────────────────────────── + +/// Configuration for [`MultiAlgorithmHasher`]. +/// +/// All fields have sensible defaults via [`Default`]. Callers may override +/// with the `with_*` builder methods, which return `Self` by value. +#[derive(Debug, Clone)] +pub struct HasherConfig { + /// Algorithms to compute for every hash request. SHA-256 is always + /// included even if absent from this list. BLAKE3 is included by default + /// in [`HasherConfig::default`]. + pub algorithms: Vec, + /// Maximum number of concurrent hash operations across all callers. + /// Defaults to `clamp(available_parallelism(), 2, 16)`. + pub max_concurrent: usize, + /// Maximum accepted file size. Files larger than this are rejected with + /// [`HashError::FileTooLarge`] before any bytes are read. + pub max_file_size: u64, + /// Per-file hash deadline. Enforced cooperatively via an `AtomicBool` + /// cancel flag checked inside the streaming read loop. + pub timeout_per_file: Duration, + /// Streaming read buffer size. Defaults to [`BUFFER_SIZE`]. + pub buffer_size: usize, + /// Bounded-cache capacity in entries. Zero disables the cache. + pub cache_capacity: usize, +} + +impl Default for HasherConfig { + fn default() -> Self { + let max_concurrent = std::thread::available_parallelism() + .map(std::num::NonZero::get) + .unwrap_or(MIN_CONCURRENCY) + .clamp(MIN_CONCURRENCY, MAX_CONCURRENCY); + Self { + algorithms: vec![HashAlgorithm::Sha256, HashAlgorithm::Blake3], + max_concurrent, + max_file_size: DEFAULT_MAX_FILE_SIZE, + timeout_per_file: DEFAULT_TIMEOUT, + buffer_size: BUFFER_SIZE, + cache_capacity: DEFAULT_CACHE_CAPACITY, + } + } +} + +impl HasherConfig { + /// Builder: replace the algorithm list entirely. + #[must_use] + pub fn with_algorithms(mut self, algorithms: Vec) -> Self { + self.algorithms = algorithms; + self + } + + /// Builder: set `max_concurrent`, clamped to `[MIN_CONCURRENCY, MAX_CONCURRENCY]`. + #[must_use] + pub fn with_max_concurrent(mut self, n: usize) -> Self { + self.max_concurrent = n.clamp(MIN_CONCURRENCY, MAX_CONCURRENCY); + self + } + + /// Builder: set the maximum file size accepted for hashing. + #[must_use] + pub const fn with_max_file_size(mut self, bytes: u64) -> Self { + self.max_file_size = bytes; + self + } + + /// Builder: set the per-file timeout. + #[must_use] + pub const fn with_timeout(mut self, timeout: Duration) -> Self { + self.timeout_per_file = timeout; + self + } + + /// Builder: set the bounded cache capacity (0 = disabled). + #[must_use] + pub const fn with_cache_capacity(mut self, n: usize) -> Self { + self.cache_capacity = n; + self + } + + /// Validate the configuration, returning a descriptive error on failure. + /// + /// # Errors + /// + /// Returns [`HashError::InvalidConfig`] if any field is out of range. + pub fn validate(&self) -> Result<(), HashError> { + if self.algorithms.is_empty() { + return Err(HashError::InvalidConfig( + "algorithms list must not be empty".to_owned(), + )); + } + if self.max_concurrent == 0 { + return Err(HashError::InvalidConfig( + "max_concurrent must be greater than zero".to_owned(), + )); + } + if self.max_file_size == 0 { + return Err(HashError::InvalidConfig( + "max_file_size must be greater than zero".to_owned(), + )); + } + if self.buffer_size == 0 { + return Err(HashError::InvalidConfig( + "buffer_size must be greater than zero".to_owned(), + )); + } + if self.timeout_per_file.is_zero() { + return Err(HashError::InvalidConfig( + "timeout_per_file must be greater than zero".to_owned(), + )); + } + Ok(()) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// HashComputer trait +// ───────────────────────────────────────────────────────────────────────────── + +/// Abstract interface for a streaming, multi-algorithm hash engine. +/// +/// Implementations must be `Send + Sync` so a single engine can be shared via +/// `Arc` between the inline enumeration path and the on-demand triggered +/// collector. +/// +/// This trait uses native `async fn` + `#[allow(async_fn_in_trait)]` per +/// workspace convention (see `EventSource` at `collector-core/src/source.rs`). +#[allow(async_fn_in_trait)] +pub trait HashComputer: Send + Sync { + /// Compute hashes for the file at `path`, respecting the configured + /// deadline, concurrency cap, and file-size limit. + /// + /// # Errors + /// + /// Returns [`HashError`] on I/O failure, timeout, cancellation, + /// oversized files, or permission errors. + async fn compute(&self, path: &Path) -> Result; + + /// Return the algorithms this computer will emit on every successful + /// `compute` call (excluding legacy algorithms which land in a separate + /// bucket on `HashResult`). + fn supported_algorithms(&self) -> &[HashAlgorithm]; + + /// Optional health check. The default implementation is a no-op. + /// + /// # Errors + /// + /// Implementations may return [`HashError`] to signal unhealthy state. + async fn health_check(&self) -> Result<(), HashError> { + Ok(()) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Enum-dispatched multi-hasher +// ───────────────────────────────────────────────────────────────────────────── + +/// Enum wrapper over the concrete per-algorithm hashers. +/// +/// We deliberately do **not** use `Box` here: enabling BLAKE3's +/// `traits-preview` feature to expose `digest::Digest` creates method-resolution +/// conflicts with existing `sha2::Digest` usage elsewhere in the workspace. +/// Enum dispatch is strictly simpler, avoids the conflict, and has negligible +/// overhead (one match per 256 KiB chunk). +enum HasherKind { + Sha256(Box), + Blake3(Box), + #[cfg(feature = "sha3-hashes")] + Sha3_256(Box), +} + +impl HasherKind { + fn new(algorithm: HashAlgorithm) -> Self { + match algorithm { + HashAlgorithm::Sha256 => Self::Sha256(Box::new(Sha256::new())), + HashAlgorithm::Blake3 => Self::Blake3(Box::new(blake3::Hasher::new())), + #[cfg(feature = "sha3-hashes")] + HashAlgorithm::Sha3_256 => Self::Sha3_256(Box::new(Sha3_256::new())), + } + } + + const fn algorithm(&self) -> HashAlgorithm { + match *self { + Self::Sha256(_) => HashAlgorithm::Sha256, + Self::Blake3(_) => HashAlgorithm::Blake3, + #[cfg(feature = "sha3-hashes")] + Self::Sha3_256(_) => HashAlgorithm::Sha3_256, + } + } + + fn update(&mut self, data: &[u8]) { + match *self { + Self::Sha256(ref mut h) => Sha2Digest::update(h.as_mut(), data), + Self::Blake3(ref mut h) => { + // Inherent method returns &mut Self for chaining; we discard. + let _ = h.update(data); + } + #[cfg(feature = "sha3-hashes")] + Self::Sha3_256(ref mut h) => { + use sha3::Digest; + h.as_mut().update(data); + } + } + } + + fn finalize_hex(self) -> String { + match self { + Self::Sha256(h) => bytes_to_hex(Sha2Digest::finalize(*h).as_slice()), + Self::Blake3(h) => h.finalize().to_hex().to_string(), + #[cfg(feature = "sha3-hashes")] + Self::Sha3_256(h) => { + use sha3::Digest; + bytes_to_hex((*h).finalize().as_slice()) + } + } + } +} + +/// Encode a byte slice as a lowercase hex string. +/// +/// Used instead of `format!("{:x}", ...)` because sha2 0.11 returns +/// `hybrid_array::Array` which does not implement `LowerHex`. +fn bytes_to_hex(bytes: &[u8]) -> String { + const HEX_CHARS: [u8; 16] = *b"0123456789abcdef"; + let mut out = String::with_capacity(bytes.len().saturating_mul(2)); + for &b in bytes { + // u8 >> 4 and u8 & 0x0f are both in [0, 16), so indexing a 16-element + // array is guaranteed in-bounds at the type level. + let hi = (b >> 4) & 0x0f; + let lo = b & 0x0f; + // Debug-only invariant check: both nibbles must be < 16 so that the + // get() calls below never hit the fallback branch. + debug_assert!( + usize::from(hi) < HEX_CHARS.len() && usize::from(lo) < HEX_CHARS.len(), + "hex nibble out of range" + ); + // `HEX_CHARS[idx]` where idx < 16 cannot panic; use get() + fallback + // for belt-and-braces under clippy::indexing_slicing = "warn". + out.push(char::from(*HEX_CHARS.get(usize::from(hi)).unwrap_or(&b'0'))); + out.push(char::from(*HEX_CHARS.get(usize::from(lo)).unwrap_or(&b'0'))); + } + out +} + +/// Fan-out structure holding one hasher per configured algorithm. +struct HasherSet { + hashers: Vec, +} + +impl HasherSet { + fn new(algorithms: &[HashAlgorithm]) -> Self { + // The caller (`MultiAlgorithmHasher`) normalizes its config to + // guarantee SHA-256 is always present at the head of `algorithms`, + // so we only need to deduplicate here — not inject SHA-256. + let mut seen: Vec = Vec::with_capacity(algorithms.len()); + for a in algorithms { + if !seen.contains(a) { + seen.push(*a); + } + } + Self { + hashers: seen.into_iter().map(HasherKind::new).collect(), + } + } + + fn update(&mut self, data: &[u8]) { + for h in &mut self.hashers { + h.update(data); + } + } + + fn finalize_into(self) -> BTreeMap { + let mut secure: BTreeMap = BTreeMap::new(); + for h in self.hashers { + let algo = h.algorithm(); + // All engine-supported algorithms are cryptographically secure + // by construction (SHA-1 and MD5 are not compiled in). We still + // assert it at runtime as a defence-in-depth invariant so any + // future addition of a weak algorithm to the engine would trip + // this check before producing a trusted-looking result. + debug_assert!( + algo.is_cryptographically_secure(), + "HasherSet must only produce cryptographically secure hashes" + ); + let hex = h.finalize_hex(); + let _ = secure.insert(algo, hex); + } + secure + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// MultiAlgorithmHasher +// ───────────────────────────────────────────────────────────────────────────── + +// NOTE: cache key uses (path, modified_time, file_size, identity). +// +// SystemTime resolution is platform-dependent — nanoseconds on Linux and +// APFS, 100ns on Windows, but historically 1s on HFS+. On a 1s-resolution +// filesystem, two writes to the same file within one second with identical +// size could otherwise return a stale cached hash. +// +// More importantly, (path, mtime, size) alone is **not** strong enough for +// a security-sensitive hasher. An attacker who replaces a file with a +// different inode — but preserves (mtime, size) at the same path — could +// otherwise hit a stale cache entry and be returned the wrong digest. The +// concern is acute for `compute_from_file_with_deadline`: the caller holds +// an authorized fd for a specific inode, but without an identity component +// a cache hit could return the digest of a DIFFERENT inode that happened +// to live at this path earlier with the same mtime+size. +// +// `FileIdentity` closes that gap by binding the key to the underlying +// file identity: (dev, ino) on Unix, and a unit sentinel on non-Unix +// platforms. Combined with the fstat performed on the held handle, a +// cache hit on Unix is now only reachable when the same path, with the +// same mtime, size, AND underlying inode was previously hashed — i.e. +// the same file, not merely the same name. +// +// On Windows, `std::os::windows::fs::MetadataExt::file_index()` requires +// the unstable `windows_by_handle` feature and is not available on +// stable Rust. Rather than pulling in `windows-sys` just to call +// `GetFileInformationByHandle` for this, Windows currently falls back +// to the weaker (path, mtime, size) key. The TOCTOU-sensitive callers +// (cap-std `BinaryHasherCollector` and procmond's `O_NOFOLLOW`-opened +// enumeration path) still hash from a held fd, so an attacker racing +// the path after authorization cannot change the bytes we actually +// digest — they could only influence a cache lookup, which is already +// disabled when `cache_capacity == 0`. Security-critical Windows +// deployments should set `cache_capacity = 0` until a proper +// file-index probe lands. +#[cfg(unix)] +type FileIdentity = (u64, u64); // (dev, ino) +#[cfg(not(unix))] +type FileIdentity = (); + +type CacheKey = (PathBuf, SystemTime, u64, FileIdentity); + +/// Extract a platform-appropriate file identity component from metadata +/// obtained by fstat'ing an opened handle. +#[cfg(unix)] +fn file_identity(metadata: &std::fs::Metadata) -> FileIdentity { + use std::os::unix::fs::MetadataExt; + (metadata.dev(), metadata.ino()) +} + +#[cfg(not(unix))] +fn file_identity(_metadata: &std::fs::Metadata) -> FileIdentity { + // Non-Unix platforms fall back to the weaker (path, mtime, size) key. + // See the CacheKey module comment above for the security rationale. +} + +/// The default [`HashComputer`] implementation. +/// +/// Single instance intended to be held behind `Arc` and shared between the +/// inline enumeration path (procmond) and the on-demand triggered path +/// (`BinaryHasherCollector`). Sharing the same instance guarantees **single +/// policy**: one concurrency cap, one algorithm list, one size/timeout +/// contract — no matter which caller invokes `compute`. Cross-path cache +/// hits are a small side benefit in steady state but are not the primary +/// motivation. +/// +/// # Statelessness invariant +/// +/// `MultiAlgorithmHasher` holds **no per-path state that bleeds between +/// callers**. The bounded cache is keyed by `(path, mtime, size)` triples +/// and is monotonic — a lookup never depends on who inserted the entry. +/// No telemetry, rate limiter, or error log is keyed by path at the engine +/// level. This matters because the same `Arc` backs +/// both the kernel-sourced path (procmond process enumeration) and the +/// untrusted triggered path (rule-initiated hash requests). If the engine +/// were stateful across calls, a triggered-path caller could probe for +/// sensitive file existence via timing side-channels on the shared `Arc`. +/// Any future addition to this struct must preserve the invariant or scope +/// its state per-call via an explicit context argument. +pub struct MultiAlgorithmHasher { + config: HasherConfig, + permits: Arc, + cache: Option>>>, +} + +impl MultiAlgorithmHasher { + /// Construct a new hasher from a configuration. + /// + /// The provided `config` is normalized so SHA-256 is always the first + /// algorithm in `config.algorithms`. This matches the invariant enforced + /// by the internal `HasherSet::new` — every `HashResult` produced by + /// this engine contains a SHA-256 digest regardless of what the caller + /// requested. Normalizing here ensures [`HashComputer::supported_algorithms`] + /// returns the list that actually gets computed, so callers relying on + /// that reflection do not receive a lie. + /// + /// # Errors + /// + /// Returns [`HashError::InvalidConfig`] if `config.validate()` fails. + pub fn new(mut config: HasherConfig) -> Result { + config.validate()?; + if !config.algorithms.contains(&HashAlgorithm::Sha256) { + config.algorithms.insert(0, HashAlgorithm::Sha256); + } + let permits = Arc::new(Semaphore::new(config.max_concurrent)); + let cache = + (config.cache_capacity > 0).then(|| Arc::new(Cache::new(config.cache_capacity))); + Ok(Self { + config, + permits, + cache, + }) + } + + /// Return the configured max concurrency. + #[must_use] + pub const fn max_concurrent(&self) -> usize { + self.config.max_concurrent + } + + /// Compute hashes with an explicit deadline. Most callers should use the + /// trait method [`HashComputer::compute`] instead, which derives the + /// deadline from `config.timeout_per_file`. + /// + /// This path opens `path` ambiently via `std::fs::File::open`. + /// **TOCTOU-sensitive callers** (e.g. `BinaryHasherCollector`) must + /// use [`MultiAlgorithmHasher::compute_from_file_with_deadline`] + /// instead, passing a file descriptor they authorized through a + /// cap-std `Dir` handle. + /// + /// # Errors + /// + /// Returns [`HashError`] on I/O failure, timeout, cancellation, or + /// oversized files. + pub async fn compute_with_deadline( + &self, + path: &Path, + deadline: Instant, + ) -> Result { + // Fast-fail on non-files BEFORE attempting to open. On Windows, + // `std::fs::File::open` on a directory returns `PermissionDenied` + // (CreateFile without FILE_FLAG_BACKUP_SEMANTICS), which would map + // to `HashError::PermissionDenied` — misleading for callers + // expecting "not a regular file". Pre-checking with + // `symlink_metadata` lets us surface `HashError::Io` with a clear + // message on every platform. + // + // Note: a second `fstat` still runs inside + // `compute_from_file_with_deadline` against the opened handle, so + // the authoritative size/mtime used for the cache key and the + // mid-read-mutation check are the file we actually read — not the + // path we stat'd first. This opening stat is a fast-fail only. + let pre_meta = tokio::fs::symlink_metadata(path) + .await + .map_err(|e| HashError::from_io(path, e))?; + if !pre_meta.is_file() { + return Err(HashError::Io { + path: path.to_path_buf(), + source: io::Error::other("path is not a regular file"), + }); + } + + // Open the file by path; delegate to the file-based entry point. + // This is the ambient-authority path used by procmond's + // kernel-resolved exe hashing (which has already gone through + // `authorize_kernel_path` and does not need cap-std confinement). + let file = std::fs::File::open(path).map_err(|e| HashError::from_io(path, e))?; + self.compute_from_file_with_deadline(path, file, deadline) + .await + } + + /// Compute hashes from an **already-opened** file handle, with an + /// explicit deadline. + /// + /// TOCTOU-safe callers (e.g. `BinaryHasherCollector`) that authorize + /// a path via cap-std `Dir::open` pass the resulting file descriptor + /// here so the hash reads the inode that was authorized — not a path + /// that may have been swapped between authorization and hashing. + /// + /// `path` is used **only** for the [`HashResult::file_path`] field, + /// the cache key, and error context; it is never used to (re-)open + /// the file. + /// + /// # Errors + /// + /// Returns [`HashError`] on I/O failure, timeout, cancellation, or + /// oversized files. + pub async fn compute_from_file_with_deadline( + &self, + path: &Path, + file: std::fs::File, + deadline: Instant, + ) -> Result { + // 1. fstat the handle (not the path) so mtime/size reflect the + // inode we will actually hash. + let metadata = file.metadata().map_err(|e| HashError::from_io(path, e))?; + + if !metadata.is_file() { + return Err(HashError::Io { + path: path.to_path_buf(), + source: io::Error::other("path is not a regular file"), + }); + } + + let file_size = metadata.len(); + if file_size > self.config.max_file_size { + return Err(HashError::FileTooLarge { + size: file_size, + limit: self.config.max_file_size, + }); + } + let modified_time = metadata + .modified() + .map_err(|e| HashError::from_io(path, e))?; + + // 2. Cache lookup. Cache key uses the file's fstat-derived + // (mtime, size, identity) so reopening by path cannot evade + // a hit, and a same-path/same-mtime/same-size inode swap + // cannot cause a stale hit against a different inode. + let identity = file_identity(&metadata); + let key: CacheKey = (path.to_path_buf(), modified_time, file_size, identity); + if let Some(cache) = self.cache.as_ref() + && let Some(hit) = cache.get(&key) + { + debug!(path = ?path, "integrity cache hit"); + return Ok((*hit).clone()); + } + + // 3. Acquire permit BEFORE any hashing work begins. + let permit = Arc::clone(&self.permits) + .acquire_owned() + .await + .map_err(|err| HashError::InvalidConfig(format!("semaphore closed: {err}")))?; + + // 4. Size-based dispatch. Both branches use cooperative cancellation + // even though the inline branch has no timeout racer — it keeps + // the signature uniform and lets the inner loop's deadline check + // catch a clock-already-expired case for small files too. + let algorithms = self.config.algorithms.clone(); + let buffer_size = self.config.buffer_size; + let max_file_size = self.config.max_file_size; + let cancel = Arc::new(AtomicBool::new(false)); + let path_owned = path.to_path_buf(); + + let hash_outcome = if file_size < SPAWN_BLOCKING_THRESHOLD { + // Small file: hash inline on the current task. + hash_sync( + file, + &path_owned, + deadline, + &cancel, + &algorithms, + buffer_size, + max_file_size, + ) + } else { + // Large file: spawn_blocking with cooperative cancellation. + // The `File` is moved into the blocking task so it keeps + // ownership of the fd. `std::fs::File` is `Send + 'static`. + let cancel_for_task = Arc::clone(&cancel); + let path_for_task = path_owned.clone(); + let algorithms_for_task = algorithms.clone(); + + let join = tokio::task::spawn_blocking(move || { + hash_sync( + file, + &path_for_task, + deadline, + &cancel_for_task, + &algorithms_for_task, + buffer_size, + max_file_size, + ) + }); + + let sleep = tokio::time::sleep_until(deadline.into()); + tokio::pin!(sleep); + tokio::pin!(join); + + tokio::select! { + () = &mut sleep => { + cancel.store(true, Ordering::Relaxed); + // Reap the join handle so the permit releases and the + // blocking thread is returned to the pool. + match (&mut join).await { + Ok(r) => r, + Err(e) => Err(HashError::Join(e.to_string())), + } + } + r = &mut join => { + match r { + Ok(inner) => inner, + Err(e) => Err(HashError::Join(e.to_string())), + } + } + } + }; + + drop(permit); + let hash_result = hash_outcome?; + + if let Some(cache) = self.cache.as_ref() { + cache.insert(key, Arc::new(hash_result.clone())); + } + Ok(hash_result) + } + + /// Compute hashes from an already-opened file handle, using the + /// engine's configured per-file timeout. + /// + /// TOCTOU-safe entry point used by callers that authorize a path via + /// cap-std. See [`MultiAlgorithmHasher::compute_from_file_with_deadline`] + /// for details. + /// + /// # Errors + /// + /// Returns [`HashError`] on I/O failure, timeout, cancellation, or + /// oversized files. + pub async fn compute_from_file( + &self, + path: &Path, + file: std::fs::File, + ) -> Result { + let deadline = Instant::now() + .checked_add(self.config.timeout_per_file) + .ok_or_else(|| HashError::InvalidConfig("deadline overflow".to_owned()))?; + self.compute_from_file_with_deadline(path, file, deadline) + .await + } +} + +impl HashComputer for MultiAlgorithmHasher { + async fn compute(&self, path: &Path) -> Result { + // `Instant + Duration` is infallible in release builds and saturates + // in debug — use `checked_add` so we never panic on overflow. + let deadline = Instant::now() + .checked_add(self.config.timeout_per_file) + .ok_or_else(|| HashError::InvalidConfig("deadline overflow".to_owned()))?; + self.compute_with_deadline(path, deadline).await + } + + fn supported_algorithms(&self) -> &[HashAlgorithm] { + &self.config.algorithms + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Synchronous inner hash routine +// ───────────────────────────────────────────────────────────────────────────── + +/// Synchronous streaming hash with cooperative cancellation. +/// +/// This function is the only place in the engine that actually reads from +/// disk. It is called: +/// +/// 1. Directly (inline) for files smaller than `SPAWN_BLOCKING_THRESHOLD`. +/// 2. From inside `tokio::task::spawn_blocking` for larger files. +/// +/// The caller passes an already-opened `std::fs::File`. The public entry +/// points [`MultiAlgorithmHasher::compute_with_deadline`] (path-based) and +/// [`MultiAlgorithmHasher::compute_from_file_with_deadline`] (fd-based) +/// both funnel into this routine. TOCTOU-safe callers (e.g. +/// `BinaryHasherCollector`) pass a file descriptor obtained via cap-std so +/// the hash reads the inode that was authorized, not a path that may have +/// been swapped. +/// +/// `path` is used **only** for the `HashResult::file_path` field and for +/// error context. It is never used to (re-)open the file. +/// +/// The caller passes an `Arc` cancel flag. The outer async driver +/// flips this flag when a deadline expires; the loop observes it on the next +/// iteration and returns `HashError::Cancelled` or `HashError::Timeout`. +fn hash_sync( + mut file: std::fs::File, + path: &Path, + deadline: Instant, + cancel: &AtomicBool, + algorithms: &[HashAlgorithm], + buffer_size: usize, + max_file_size: u64, +) -> Result { + let start = Instant::now(); + + let meta_before = file.metadata().map_err(|e| HashError::from_io(path, e))?; + + if !meta_before.is_file() { + return Err(HashError::Io { + path: path.to_path_buf(), + source: io::Error::new(io::ErrorKind::InvalidInput, "path is not a regular file"), + }); + } + + let file_size_before = meta_before.len(); + let modified_before = meta_before + .modified() + .map_err(|e| HashError::from_io(path, e))?; + + if file_size_before > max_file_size { + return Err(HashError::FileTooLarge { + size: file_size_before, + limit: max_file_size, + }); + } + + let mut hashers = HasherSet::new(algorithms); + let mut buf = vec![0_u8; buffer_size]; + let mut total_read: u64 = 0; + + loop { + if cancel.load(Ordering::Relaxed) { + return Err(HashError::Cancelled); + } + if Instant::now() >= deadline { + return Err(HashError::Timeout); + } + if total_read > max_file_size { + // Distinguish growth-during-read from a file that was already + // over-budget. The pre-read size check has already gated + // `file_size_before <= max_file_size`, so any `total_read` + // that exceeds the cap is either (a) the file grew while we + // were reading it — a forensic mutation signal — or (b) the + // file was exactly at the cap and the final `read()` call + // returned a stale buffer (practically impossible, but treat + // it as a configuration error to be conservative). + if total_read > file_size_before { + return Err(HashError::Nonauthoritative { + path: path.to_path_buf(), + }); + } + return Err(HashError::FileTooLarge { + size: total_read, + limit: max_file_size, + }); + } + + let n = match file.read(&mut buf) { + Ok(n) => n, + Err(ref e) if e.kind() == io::ErrorKind::Interrupted => continue, + Err(e) => return Err(HashError::from_io(path, e)), + }; + if n == 0 { + break; + } + let chunk = buf.get(..n).ok_or_else(|| HashError::Io { + path: path.to_path_buf(), + source: io::Error::other("read returned oversized count"), + })?; + // usize → u64 is a widening conversion on all supported targets + // (and saturating_add prevents any overflow regardless). + let n_u64 = u64::try_from(n).unwrap_or(u64::MAX); + total_read = total_read.saturating_add(n_u64); + hashers.update(chunk); + } + + let meta_after = file.metadata().map_err(|e| HashError::from_io(path, e))?; + let size_after = meta_after.len(); + let modified_after = meta_after.modified().ok(); + + if size_after != file_size_before || modified_after != Some(modified_before) { + warn!( + path = ?path, + "file metadata changed during hash; returning Nonauthoritative" + ); + return Err(HashError::Nonauthoritative { + path: path.to_path_buf(), + }); + } + + let hashes = hashers.finalize_into(); + + Ok(HashResult { + file_path: path.to_path_buf(), + file_size: file_size_before, + modified_time: modified_before, + hashes, + integrity: HashIntegrity::Stable, + computation_time: start.elapsed(), + }) +} + +// ───────────────────────────────────────────────────────────────────────────── +// Tests +// ───────────────────────────────────────────────────────────────────────────── + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + use super::*; + use std::fs; + use tempfile::NamedTempFile; + + // ── Algorithm metadata ────────────────────────────────────────────── + + #[test] + fn algorithm_security_classification() { + assert!(HashAlgorithm::Sha256.is_cryptographically_secure()); + assert!(HashAlgorithm::Blake3.is_cryptographically_secure()); + #[cfg(feature = "sha3-hashes")] + assert!(HashAlgorithm::Sha3_256.is_cryptographically_secure()); + } + + #[test] + fn algorithm_wire_names() { + assert_eq!(HashAlgorithm::Sha256.wire_name(), "sha256"); + assert_eq!(HashAlgorithm::Blake3.wire_name(), "blake3"); + assert_eq!(HashAlgorithm::Sha256.to_string(), "sha256"); + } + + #[test] + fn algorithm_hex_lengths() { + assert_eq!(HashAlgorithm::Sha256.hex_len(), 64); + assert_eq!(HashAlgorithm::Blake3.hex_len(), 64); + } + + // ── Config validation ─────────────────────────────────────────────── + + #[test] + fn default_config_validates() { + HasherConfig::default().validate().unwrap(); + } + + #[test] + fn default_config_defaults_to_sha256_and_blake3() { + let cfg = HasherConfig::default(); + assert!(cfg.algorithms.contains(&HashAlgorithm::Sha256)); + assert!(cfg.algorithms.contains(&HashAlgorithm::Blake3)); + } + + #[test] + fn default_config_concurrency_clamped() { + let cfg = HasherConfig::default(); + assert!(cfg.max_concurrent >= MIN_CONCURRENCY); + assert!(cfg.max_concurrent <= MAX_CONCURRENCY); + } + + #[test] + fn config_builder_clamps_concurrency() { + let high = HasherConfig::default().with_max_concurrent(9999); + assert_eq!(high.max_concurrent, MAX_CONCURRENCY); + let low = HasherConfig::default().with_max_concurrent(0); + assert_eq!(low.max_concurrent, MIN_CONCURRENCY); + } + + #[test] + fn empty_algorithms_fails_validation() { + let cfg = HasherConfig::default().with_algorithms(vec![]); + assert!(cfg.validate().is_err()); + } + + #[test] + fn zero_max_file_size_fails_validation() { + let cfg = HasherConfig::default().with_max_file_size(0); + assert!(cfg.validate().is_err()); + } + + #[test] + fn zero_timeout_fails_validation() { + let cfg = HasherConfig::default().with_timeout(Duration::ZERO); + assert!(cfg.validate().is_err()); + } + + // ── Known-answer tests (NIST + BLAKE3 official) ───────────────────── + + /// NIST FIPS 180-4: SHA-256 of empty string. + const SHA256_EMPTY: &str = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + + /// NIST FIPS 180-4: SHA-256 of "abc". + const SHA256_ABC: &str = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"; + + /// BLAKE3 official test vector: empty input. + const BLAKE3_EMPTY: &str = "af1349b9f5f9a1a6a0404dea36dcc9499bcb25c9adc112b7cc9a93cae41f3262"; + + /// BLAKE3 official test vector: "abc". + const BLAKE3_ABC: &str = "6437b3ac38465133ffb63b75273a8db548c558465d79db03fd359c6cd5bd9d85"; + + async fn hash_bytes(bytes: &[u8]) -> HashResult { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), bytes).unwrap(); + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + hasher.compute(tmp.path()).await.unwrap() + } + + #[tokio::test] + async fn sha256_empty_matches_nist_vector() { + let r = hash_bytes(b"").await; + assert_eq!(r.sha256().unwrap(), SHA256_EMPTY); + } + + #[tokio::test] + async fn sha256_abc_matches_nist_vector() { + let r = hash_bytes(b"abc").await; + assert_eq!(r.sha256().unwrap(), SHA256_ABC); + } + + #[tokio::test] + async fn blake3_empty_matches_official_vector() { + let r = hash_bytes(b"").await; + assert_eq!(r.blake3().unwrap(), BLAKE3_EMPTY); + } + + #[tokio::test] + async fn blake3_abc_matches_official_vector() { + let r = hash_bytes(b"abc").await; + assert_eq!(r.blake3().unwrap(), BLAKE3_ABC); + } + + #[tokio::test] + async fn large_file_spans_spawn_blocking_threshold() { + // 512 KiB > 256 KiB threshold — takes the spawn_blocking path. + let bytes = vec![0_u8; 512 * 1024]; + let r = hash_bytes(&bytes).await; + assert_eq!(r.sha256().unwrap().len(), 64); + assert_eq!(r.blake3().unwrap().len(), 64); + assert!(r.is_authoritative()); + assert_eq!(r.file_size, 512 * 1024); + } + + #[tokio::test] + async fn deterministic_repeated_hash_yields_same_result() { + let bytes = b"deterministic repeatability test payload"; + let r1 = hash_bytes(bytes).await; + let r2 = hash_bytes(bytes).await; + assert_eq!(r1.hashes, r2.hashes); + } + + #[tokio::test] + async fn byte_change_produces_different_hash() { + let r1 = hash_bytes(b"payload-a").await; + let r2 = hash_bytes(b"payload-b").await; + assert_ne!(r1.hashes, r2.hashes); + } + + // ── Error paths ───────────────────────────────────────────────────── + + #[tokio::test] + async fn missing_file_returns_file_not_found() { + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + let err = hasher + .compute(Path::new("/definitely/does/not/exist/xyz")) + .await + .unwrap_err(); + assert!(matches!(err, HashError::FileNotFound { .. })); + } + + #[tokio::test] + async fn oversized_file_returns_too_large() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), vec![0_u8; 4096]).unwrap(); + let cfg = HasherConfig::default().with_max_file_size(1024); + let hasher = MultiAlgorithmHasher::new(cfg).unwrap(); + let err = hasher.compute(tmp.path()).await.unwrap_err(); + let HashError::FileTooLarge { size, limit } = err else { + panic!("expected FileTooLarge, got {err:?}"); + }; + assert_eq!(size, 4096); + assert_eq!(limit, 1024); + } + + #[tokio::test] + async fn directory_rejected() { + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + let tmp = tempfile::tempdir().unwrap(); + let err = hasher.compute(tmp.path()).await.unwrap_err(); + assert!(matches!(err, HashError::Io { .. })); + } + + #[tokio::test] + async fn integrity_marked_stable_on_unmodified_file() { + let r = hash_bytes(b"stable file").await; + assert_eq!(r.integrity, HashIntegrity::Stable); + assert!(r.is_authoritative()); + } + + // ── Cache behavior ────────────────────────────────────────────────── + + #[tokio::test] + async fn cache_returns_identical_result_on_second_call() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"cache test").unwrap(); + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + let r1 = hasher.compute(tmp.path()).await.unwrap(); + let r2 = hasher.compute(tmp.path()).await.unwrap(); + assert_eq!(r1.hashes, r2.hashes); + assert_eq!(r1.file_size, r2.file_size); + } + + #[tokio::test] + async fn cache_disabled_still_works() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"no cache").unwrap(); + let cfg = HasherConfig::default().with_cache_capacity(0); + let hasher = MultiAlgorithmHasher::new(cfg).unwrap(); + let r = hasher.compute(tmp.path()).await.unwrap(); + assert!(r.sha256().is_some()); + } + + // ── Concurrency + timeout ─────────────────────────────────────────── + + #[tokio::test] + async fn timeout_fires_on_impossible_deadline() { + // Use a large file to ensure we take the spawn_blocking path and + // the read loop gets a chance to observe the timeout. + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), vec![0_u8; 2 * 1024 * 1024]).unwrap(); + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + let deadline = Instant::now(); // Already expired. + let err = hasher + .compute_with_deadline(tmp.path(), deadline) + .await + .unwrap_err(); + assert!(matches!(err, HashError::Timeout | HashError::Cancelled)); + } + + #[tokio::test] + async fn concurrent_hashes_all_succeed() { + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + let mut tasks = Vec::new(); + for i in 0_u32..8 { + let hasher_clone = Arc::clone(&hasher); + tasks.push(tokio::spawn(async move { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), format!("payload-{i}").as_bytes()).unwrap(); + hasher_clone.compute(tmp.path()).await + })); + } + for t in tasks { + let r = t.await.unwrap().unwrap(); + assert_eq!(r.sha256().unwrap().len(), 64); + } + } + + // ── compute_from_file (TOCTOU-safe entry point) ───────────────────── + + #[tokio::test] + async fn compute_from_file_matches_compute_by_path() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"toctou-safe hash").unwrap(); + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + + let by_path = hasher.compute(tmp.path()).await.unwrap(); + let file = std::fs::File::open(tmp.path()).unwrap(); + let by_file = hasher.compute_from_file(tmp.path(), file).await.unwrap(); + + assert_eq!(by_path.hashes, by_file.hashes); + assert_eq!(by_path.file_size, by_file.file_size); + } + + #[tokio::test] + async fn compute_from_file_hashes_authorized_inode_even_if_path_swapped() { + // Simulates the TOCTOU scenario: authorize and open a file, then + // replace the path's contents before hashing. The engine must + // hash the inode we opened, not the new contents. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("target"); + fs::write(&path, b"original authorized content").unwrap(); + + // Open BEFORE the swap (this is what cap-std does during authorization). + let file = std::fs::File::open(&path).unwrap(); + + // Attacker swaps the path contents (new inode via rename-over). + let decoy = dir.path().join("decoy"); + fs::write(&decoy, b"attacker-injected content").unwrap(); + std::fs::rename(&decoy, &path).unwrap(); + + let hasher = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + let result = hasher.compute_from_file(&path, file).await.unwrap(); + + // The hash must match the ORIGINAL content we held an fd to. + let expected = MultiAlgorithmHasher::new(HasherConfig::default()).unwrap(); + let control_tmp = NamedTempFile::new().unwrap(); + fs::write(control_tmp.path(), b"original authorized content").unwrap(); + let control = expected.compute(control_tmp.path()).await.unwrap(); + assert_eq!( + result.sha256(), + control.sha256(), + "compute_from_file must hash the held inode, not re-open by path" + ); + } + + // ── BTreeMap iteration order is stable ────────────────────────────── + + #[test] + fn btreemap_iteration_is_deterministic() { + let mut m1: BTreeMap = BTreeMap::new(); + let _ = m1.insert(HashAlgorithm::Blake3, "b".to_owned()); + let _ = m1.insert(HashAlgorithm::Sha256, "a".to_owned()); + let keys: Vec<_> = m1.keys().copied().collect(); + // Order derives from HashAlgorithm discriminant: Sha256 < Blake3. + assert_eq!(keys, vec![HashAlgorithm::Sha256, HashAlgorithm::Blake3]); + } +} diff --git a/daemoneye-lib/src/lib.rs b/daemoneye-lib/src/lib.rs index a98368a1..e65a44ed 100644 --- a/daemoneye-lib/src/lib.rs +++ b/daemoneye-lib/src/lib.rs @@ -16,6 +16,8 @@ // Core modules (always available) pub mod config; pub mod crypto; +/// Cryptographic integrity verification primitives, hashing engines, and path authorization checks. +pub mod integrity; pub mod ipc; pub mod models; pub mod proto; diff --git a/docs/solutions/security-issues/binary-hashing-authorization-and-toctou-fixes.md b/docs/solutions/security-issues/binary-hashing-authorization-and-toctou-fixes.md new file mode 100644 index 00000000..47f6191b --- /dev/null +++ b/docs/solutions/security-issues/binary-hashing-authorization-and-toctou-fixes.md @@ -0,0 +1,161 @@ +--- +title: 'Binary Hashing P1 Blockers: Composition Root, Authorization Bypass, Serial Bottleneck, TOCTOU Race (Issue #40)' +category: security-issues +date: 2026-04-09 +tags: + - rust + - binary-hashing + - toctou + - authorization + - concurrency + - composition-root + - cap-std + - newtype-pattern + - type-state + - cwe-135 +module: procmond / daemoneye-lib (integrity) / collector-core (binary_hasher) +symptom: | + (1) --compute-hashes flag was a silent no-op; + (2) populate_executable_hashes hashed arbitrary paths without authorization; + (3) executable hashing ran serially, bottlenecking collection; + (4) BinaryHasherCollector used canonicalize() + File::open() with a TOCTOU race window. +root_cause: |- + (1) No composition site in main.rs constructed Arc and injected it into collectors; + (2) No authorization boundary at the hash engine — any path was accepted, FileChanged integrity variants passed through unchecked; + (3) Sequential iteration over unique paths with no concurrency primitive; + (4) canonicalize() resolves at one point in time, File::open() opens at a later point — attacker can swap target between the two calls. +--- + +# Binary Hashing P1 Blockers: Authorization, TOCTOU, Composition Root, Serial Bottleneck + +## Problem + +Four P1 block-merge defects in the binary hashing subsystem (issue #40) prevented the feature branch from merging: + +1. **Todo #013 (CRITICAL)** — `--compute-hashes` was a silent no-op. No composition site constructed or injected a `MultiAlgorithmHasher`. +2. **Todo #011 (HIGH)** — `populate_executable_hashes` hashed every path without authorization checks. `HashIntegrity::FileChanged` results leaked through the `Ok` path. +3. **Todo #010 (HIGH)** — Hash pass ran serially: 300 unique executables x 50ms = 15s vs 1.9s with 8-way parallelism. +4. **Todo #002 (HIGH)** — `BinaryHasherCollector` used `canonicalize()` + `File::open()` with a TOCTOU race window between resolution and open. + +## Root Cause + +**#013**: `procmond/src/main.rs` parsed `--compute-hashes` but never constructed `Arc`. `ProcessEventSource` had no `hasher` field at all. The flag controlled nothing. + +**#011**: The post-enumeration hash pass fed raw sysinfo paths directly to `hasher.compute()` with no allow-list, symlink rejection, traversal check, path length cap, or file-type validation. Additionally, `HashIntegrity::FileChanged` (mid-read mutation) was represented as a low-confidence variant in `Ok(HashResult)`, meaning any caller accessing `hash_result.hashes` bypassed the integrity tag. + +**#010**: `populate_executable_hashes` iterated unique paths with sequential `.await` calls. The `Arc` inside `MultiAlgorithmHasher` was designed for parallelism but unused on this code path. + +**#002**: Two separate syscalls (`canonicalize` then `open`) with an exploitable window between them. An attacker controlling a process path could swap a file between the two calls. + +## Solution + +### Phase 1A: Type-state at the engine boundary + +Deleted `HashIntegrity::FileChanged` from the `Ok` path. When `hash_sync` detects `(size, mtime)` drift between pre-read and post-read snapshots, it returns `Err(HashError::Nonauthoritative { path })`. An `Ok(HashResult)` is now always authoritative by construction. + +**Files**: `daemoneye-lib/src/integrity/mod.rs`, `collector-core/src/binary_hasher.rs` + +### Phase 1B: Composition root wiring + +`procmond/src/main.rs` constructs exactly one `Arc` when `--compute-hashes` is set. Cloned into `ProcessEventSource` and `ProcmondMonitorCollector` via `with_hasher()` builders. Startup assertions prevent silent regression: + +```rust +assert_eq!( + cli.compute_hashes, + collector.hasher().is_some(), + "--compute-hashes={} but collector hasher={:?}; wiring broken", + cli.compute_hashes, collector.hasher().is_some() +); +``` + +Integration test `hash_composition.rs` asserts `Arc::ptr_eq` across both holders. + +**Files**: `procmond/src/main.rs`, `procmond/src/event_source.rs`, `procmond/src/monitor_collector.rs` + +### Phase 2: Authorization layer + parallel hash pass + +Created `daemoneye-lib/src/integrity/auth.rs` with shared predicates: + +- `MAX_EXECUTABLE_PATH_LEN = 4096` (corrected from 107 — the prior value conflated `sockaddr_un.sun_path` with filesystem `PATH_MAX`) +- `check_path_length` — byte-length only via `as_os_str().len()`, never slices (CWE-135) +- `check_no_traversal` — rejects `..` components +- `check_regular_file`, `check_size` — pre-open gates +- `bytes_safe_display` — truncates at valid UTF-8 char boundaries using `take_while`, never string slicing + +Created `procmond/src/hash_pass.rs`: + +- `KernelResolvedExe` newtype with private constructor — only sysinfo's `Process::exe()` output can reach `authorize_kernel_path` +- `populate_hashes` using `futures::stream::buffer_unordered(engine.max_concurrent())` +- `HashPassStats` with `auth_failures`, `nonauthoritative`, `io_failures` counters + +**Files**: `daemoneye-lib/src/integrity/auth.rs`, `procmond/src/hash_pass.rs` + +### Phase 3: cap-std TOCTOU-safe opens + +Added `AllowedRoot` struct to `collector-core/src/binary_hasher.rs`: + +- cap-std `Dir` handle opened at construction time (before privilege drop) +- On macOS: `(st_dev, st_ino)` fingerprint for bind-mount detection +- Stores both canonical and original paths (macOS `/var` -> `/private/var`) + +`authorize_confined_path` free function opens files relative to `Dir` handles — the kernel resolves the path atomically against the handle's subtree, eliminating the TOCTOU gap. + +Pinned `cap-std = "=4.0.2"` and `cap-fs-ext = "=4.0.2"` in workspace. + +**Files**: `collector-core/src/binary_hasher.rs`, `Cargo.toml`, `collector-core/Cargo.toml` + +### Phase 4: Telemetry + +`#[instrument]` span on `populate_hashes`. Info-level coverage stats emitted per pass. Startup log includes algorithm list. + +## Key Gotchas + +**macOS `/var` -> `/private/var`**: `std::fs::canonicalize("/var/folders/...")` returns `/private/var/folders/...`. Process paths from sysinfo may carry either form. `AllowedRoot` stores both and tries both in `strip_prefix`. Failing to handle this causes `PathNotAllowed` for all macOS temp-dir executables. + +**cap-std escape detection by string match**: cap-std does not expose a typed error for escapes. The sentinel `"a path led outside of the filesystem"` is stored as `CAP_STD_ESCAPE_MESSAGE` and regression-tested. Any upstream version bump that changes this message silently breaks escape detection. + +**cap-std absolute symlink rejection**: `Dir::open()` on a relative path that is an absolute symlink may be rejected because the absolute target escapes the Dir sandbox. Use relative symlinks in tests. In production, `symlink_metadata` pre-check (when `follow_symlinks=false`) catches this before the open. + +**CWE-135 byte-safety**: `MAX_EXECUTABLE_PATH_LEN` is compared against `path.as_os_str().len()` (byte length). `bytes_safe_display` uses char-aware truncation. Never use `&str[..n]` indexing on paths. Prior learning documented in `docs/solutions/best-practices/rust-security-batch-cleanup-patterns-2026-04-04.md`. + +**`tokio::time::timeout` cannot cancel `spawn_blocking`**: The hash engine uses an `AtomicBool` cancellation flag inside the blocking read loop. Timeout alone would let the thread continue holding its semaphore permit. + +## Prevention Strategies + +### Newtype authorization receipts + +The most transferable pattern: a newtype with a private constructor that can only be obtained by passing through a specific gate. The downstream function accepts only the newtype, making the gate mandatory by construction. + +```rust +pub struct KernelResolvedExe(PathBuf); +impl KernelResolvedExe { + pub(crate) const fn from_sysinfo_exe(path: PathBuf) -> Self { Self(path) } +} +// hash_one() only accepts &KernelResolvedExe — cannot be called with raw &Path +``` + +### Startup invariant assertions + +Every feature flag that controls a behavior path must have a corresponding assertion that verifies the behavioral consequence is wired, not merely that the flag was parsed. + +### Handle-based filesystem access + +Treat `canonicalize()` + `open()` as a code smell. Replace with `Dir` handle opened at startup; all subsequent operations are relative to the handle. The kernel enforces confinement. + +### Architecture review checklist additions + +1. For every feature flag: "What startup assertion verifies it is wired?" +2. For every collection with I/O: "Why is sequential acceptable, or where is `buffer_unordered`?" +3. For every `canonicalize()` call: "Is there an `open()` following it? Replace with handle-based access." +4. For every path length comparison: "Byte length or char count, and which limit applies?" +5. For every path prefix match: "Are both original and canonical forms tried?" + +## Cross-References + +- CWE-135 learning: `docs/solutions/best-practices/rust-security-batch-cleanup-patterns-2026-04-04.md` +- Engine TOCTOU-safe entry point: `daemoneye-lib/src/integrity/mod.rs` — `MultiAlgorithmHasher::compute_from_file` +- Cap-std TOCTOU defense module docs: `collector-core/src/binary_hasher.rs` module header +- Shared auth predicates module: `daemoneye-lib/src/integrity/auth.rs` +- Kernel-resolved hash pass: `procmond/src/hash_pass.rs` + +The original in-flight plan and todo tracker used during this PR lived under `docs/plans/` and `todos/`, both of which are gitignored — consult the PR description and the commit log of issue #40 for the full historical context if you need it. diff --git a/procmond/Cargo.toml b/procmond/Cargo.toml index 02050cd9..7acfe2fc 100644 --- a/procmond/Cargo.toml +++ b/procmond/Cargo.toml @@ -42,6 +42,7 @@ collector-core = { workspace = true } daemoneye-eventbus = { workspace = true } daemoneye-lib = { workspace = true } +futures = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } @@ -55,6 +56,8 @@ uuid = { workspace = true } # Platform-specific dependencies [target.'cfg(unix)'.dependencies] +# libc exposes O_NOFOLLOW / ELOOP for TOCTOU-safe exe open in hash_pass. +libc = { workspace = true } uzers = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] diff --git a/procmond/benches/bench_helpers.rs b/procmond/benches/bench_helpers.rs index d3c60500..44c24ecc 100644 --- a/procmond/benches/bench_helpers.rs +++ b/procmond/benches/bench_helpers.rs @@ -37,7 +37,10 @@ pub fn create_test_event(pid: u32) -> ProcessEvent { start_time: Some(now), cpu_usage: Some(1.5 + (pid as f64 * 0.1) % 10.0), memory_usage: Some(1_048_576_u64.saturating_add((pid as u64).saturating_mul(4096))), - executable_hash: Some(format!("hash_{:08x}", pid)), + // Use a realistic 64-hex-char SHA-256 string (no algorithm prefix) to + // match production wire format produced by `hash_pass::primary_hash_hex`. + executable_hash: Some("0".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -58,6 +61,7 @@ pub fn create_minimal_event(pid: u32) -> ProcessEvent { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -83,10 +87,10 @@ pub fn create_large_event(pid: u32) -> ProcessEvent { start_time: Some(now), cpu_usage: Some(99.9), memory_usage: Some(1_073_741_824), // 1 GB - executable_hash: Some(format!( - "sha256:{}", - "a".repeat(64) // Realistic SHA-256 length - )), + // Realistic 64-hex-char SHA-256 string (no algorithm prefix), matching + // the production wire format produced by `hash_pass::primary_hash_hex`. + executable_hash: Some("a".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("root".to_owned()), accessible: true, file_exists: true, diff --git a/procmond/benches/process_collector_benchmarks.rs b/procmond/benches/process_collector_benchmarks.rs index c2504261..b94631c2 100644 --- a/procmond/benches/process_collector_benchmarks.rs +++ b/procmond/benches/process_collector_benchmarks.rs @@ -94,6 +94,7 @@ impl BenchmarkProcessCollector { cpu_usage: Some(1.5 + (index as f64 * 0.1) % 10.0), memory_usage: Some(1_048_576_u64.saturating_add((index as u64).saturating_mul(4096))), executable_hash: Some(format!("hash_{:08x}", index)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, diff --git a/procmond/examples/process_collector_usage.rs b/procmond/examples/process_collector_usage.rs index 06b996a9..2b6be0f3 100644 --- a/procmond/examples/process_collector_usage.rs +++ b/procmond/examples/process_collector_usage.rs @@ -78,6 +78,7 @@ impl ProcessCollector for ExampleProcessCollector { cpu_usage: Some(0.1), memory_usage: Some(1024 * 1024), executable_hash: None, + hash_algorithm: None, user_id: Some("0".to_string()), accessible: true, file_exists: true, @@ -109,6 +110,7 @@ impl ProcessCollector for ExampleProcessCollector { cpu_usage: Some(0.1), memory_usage: Some(1024 * 1024), executable_hash: None, + hash_algorithm: None, user_id: Some("0".to_string()), accessible: true, file_exists: true, diff --git a/procmond/src/event_bus_connector.rs b/procmond/src/event_bus_connector.rs index ff8469db..1f9549a7 100644 --- a/procmond/src/event_bus_connector.rs +++ b/procmond/src/event_bus_connector.rs @@ -52,6 +52,7 @@ //! cpu_usage: None, //! memory_usage: None, //! executable_hash: None, +//! hash_algorithm: None, //! user_id: Some("1000".to_string()), //! accessible: true, //! file_exists: true, @@ -593,6 +594,7 @@ impl EventBusConnector { /// cpu_usage: None, /// memory_usage: None, /// executable_hash: None, + /// hash_algorithm: None, /// user_id: None, /// accessible: true, /// file_exists: true, @@ -1277,6 +1279,7 @@ mod tests { cpu_usage: Some(5.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -1573,6 +1576,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -1607,6 +1611,7 @@ mod tests { cpu_usage: Some(1.0), memory_usage: Some(1024), executable_hash: Some("hash".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -2158,6 +2163,7 @@ mod tests { cpu_usage: Some(50.5), memory_usage: Some(1024 * 1024 * 100), executable_hash: Some("sha256:abc".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("root".to_owned()), accessible: true, file_exists: true, @@ -2194,6 +2200,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: false, file_exists: false, @@ -2258,6 +2265,7 @@ mod tests { cpu_usage: Some(1.0), memory_usage: Some(1024), executable_hash: Some("hash".to_owned()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_owned()), accessible: true, file_exists: true, @@ -2404,6 +2412,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -2421,6 +2430,7 @@ mod tests { cpu_usage: Some(1.0), memory_usage: Some(1024), executable_hash: Some("d".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("e".repeat(32)), accessible: true, file_exists: true, diff --git a/procmond/src/event_source.rs b/procmond/src/event_source.rs index 31aacded..b00e5b8a 100644 --- a/procmond/src/event_source.rs +++ b/procmond/src/event_source.rs @@ -4,12 +4,13 @@ //! and wraps the existing `ProcessMessageHandler` to integrate with the collector-core //! framework while preserving all existing functionality. +use crate::hash_pass::populate_hashes; use crate::process_collector::{ FallbackProcessCollector, ProcessCollectionConfig, ProcessCollector, SysinfoProcessCollector, }; use async_trait::async_trait; use collector_core::{CollectionEvent, EventSource, SourceCaps}; -use daemoneye_lib::{storage, telemetry::PerformanceTimer}; +use daemoneye_lib::{integrity::MultiAlgorithmHasher, storage, telemetry::PerformanceTimer}; use std::sync::{ Arc, atomic::{AtomicBool, AtomicU64, AtomicUsize, Ordering}, @@ -101,6 +102,11 @@ pub struct ProcessEventSource { stats: ProcessSourceStats, /// Backpressure semaphore for event flow control backpressure_semaphore: Arc, + /// Shared executable-hash engine. `None` means `--compute-hashes` is + /// disabled; `Some` means the post-enumeration hash pass will run. + /// The engine is shared via `Arc` with the actor-mode holder so that + /// combined concurrent operations always respect a single policy. + hasher: Option>, } /// Runtime statistics for process event source monitoring. @@ -224,6 +230,7 @@ impl ProcessEventSource { collector, stats: ProcessSourceStats::default(), backpressure_semaphore, + hasher: None, } } @@ -271,9 +278,37 @@ impl ProcessEventSource { collector, stats: ProcessSourceStats::default(), backpressure_semaphore, + hasher: None, } } + /// Inject a shared executable-hash engine. + /// + /// The engine is used to populate `executable_hash` and + /// `hash_algorithm` fields on every collected `ProcessEvent` via a + /// post-enumeration pass. Callers pass the same `Arc` clone to + /// every holder in the process so that the engine's concurrency cap, + /// algorithm list, and size budget apply as a single policy. + /// + /// Construction is guarded at the composition root + /// (`procmond/src/main.rs`): when `--compute-hashes == true`, one + /// `Arc` is built and handed to every holder. + #[must_use] + pub fn with_hasher(mut self, hasher: Option>) -> Self { + self.hasher = hasher; + self + } + + /// Returns a clone of the shared hasher `Arc`, if any. + /// + /// Intended for integration tests that need to assert + /// `Arc::ptr_eq` across multiple holders. Production code should not + /// read the engine through this accessor. + #[must_use] + pub fn hasher(&self) -> Option> { + self.hasher.as_ref().map(Arc::clone) + } + /// Creates a new process event source with a custom collector and configuration. /// /// This method allows for dependency injection of different `ProcessCollector` @@ -315,6 +350,7 @@ impl ProcessEventSource { collector, stats: ProcessSourceStats::default(), backpressure_semaphore, + hasher: None, } } @@ -481,7 +517,7 @@ impl ProcessEventSource { ) .await; - let (process_events, collection_stats) = match enumeration_result { + let (mut process_events, collection_stats) = match enumeration_result { Ok(Ok((events, stats))) => (events, stats), Ok(Err(e)) => { error!(error = %e, "Process enumeration failed"); @@ -501,6 +537,96 @@ impl ProcessEventSource { } }; + // Fail-loud check: `compute_executable_hashes = true` requires + // an injected hasher. Without both, events ship with `None` hashes + // forever — another silent no-op. We log on every scan (not only + // once) so the mismatch can't be swallowed by a short test run. + if self.config.compute_executable_hashes && self.hasher.is_none() { + warn!( + "compute_executable_hashes=true but no hasher is injected via .with_hasher(); \ + events will ship with hash_algorithm=None and no hashing will occur" + ); + } + + // Populate `executable_hash` via post-enumeration pass when a + // shared hash engine is attached. The pass deduplicates unique + // paths so the per-unique-file hash cost is incurred once even + // if many processes share an executable. Failures are logged + // but non-fatal — missing hashes are represented as `None`. + // + // The hash pass is bounded by the REMAINING portion of the + // overall `collection_timeout` budget, not a fresh fixed window. + // This preserves the contract that one `collect_processes` cycle + // (enumeration + hash pass) never runs past `collection_timeout`, + // which keeps scheduling stable under load. If enumeration + // consumed the entire budget, we skip the hash pass entirely + // rather than starve the interval. On timeout, partial coverage + // is logged and kept — downstream handles missing hashes as + // `None`. + // + // Also shutdown-aware: if `shutdown_signal` flips during the + // hash pass, we race `populate_hashes` against a cancellation + // poller so a stalled filesystem cannot delay `stop()` for up + // to the full remaining budget. + if let Some(ref hasher) = self.hasher { + if shutdown_signal.load(Ordering::Relaxed) { + debug!("shutdown requested, skipping hash population"); + } else { + let remaining_budget = self + .config + .collection_timeout + .saturating_sub(collection_start.elapsed()); + if remaining_budget.is_zero() { + warn!( + collection_timeout_secs = self.config.collection_timeout.as_secs(), + "skipping hash population: collection_timeout budget exhausted by enumeration" + ); + } else { + let hash_future = populate_hashes(&mut process_events, hasher); + let shutdown_watcher = { + let signal = Arc::clone(shutdown_signal); + async move { + loop { + if signal.load(Ordering::Relaxed) { + return; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + } + }; + tokio::select! { + biased; + () = shutdown_watcher => { + warn!( + "shutdown signaled during hash population; \ + in-flight outcomes discarded, committed outcomes preserved" + ); + } + hash_result = tokio::time::timeout(remaining_budget, hash_future) => { + match hash_result { + Ok(hash_stats) => { + debug!( + unique_paths = hash_stats.unique_paths, + hashed = hash_stats.hashed, + auth_failures = hash_stats.auth_failures, + io_failures = hash_stats.io_failures, + nonauthoritative = hash_stats.nonauthoritative, + "executable hash pass completed" + ); + } + Err(_elapsed) => { + warn!( + remaining_budget_ms = remaining_budget.as_millis(), + "hash population exceeded remaining collection budget; partial coverage recorded" + ); + } + } + } + } + } + } + } + debug!( total_found = collection_stats.total_processes, successful = collection_stats.successful_collections, @@ -1272,6 +1398,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: false, @@ -1288,6 +1415,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: false, @@ -1398,6 +1526,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: false, diff --git a/procmond/src/hash_pass.rs b/procmond/src/hash_pass.rs new file mode 100644 index 00000000..3e1275f2 --- /dev/null +++ b/procmond/src/hash_pass.rs @@ -0,0 +1,787 @@ +//! Post-enumeration executable-hash pass for procmond. +//! +//! This module provides [`populate_hashes`], the parallel, authorization-gated +//! replacement for the serial `populate_executable_hashes` in +//! `process_collector.rs`. It bundles two P1 resolutions: +//! +//! - **todo #011**: Authorization check before every hash. The free function +//! [`authorize_kernel_path`] validates paths supplied by sysinfo's +//! kernel-resolved `Process::exe()` — never argv\[0\], cwd, or root. +//! - **todo #010**: Serial bottleneck. `populate_hashes` uses +//! [`futures::stream::iter`] + [`futures::stream::StreamExt::buffer_unordered`] +//! with `n = engine.max_concurrent()` so up to N hashes run concurrently. +//! +//! # Trust model +//! +//! Procmond runs elevated and feeds sysinfo's `exe()` field, which reads +//! `/proc/\[pid\]/exe` on Linux (a kernel symlink) or `PROC_PIDPATHINFO` on +//! macOS. These are kernel-resolved paths — not user-controllable. The +//! [`KernelResolvedExe`] newtype enforces this at the type level. + +use collector_core::ProcessEvent; +use daemoneye_lib::integrity::{ + HashAlgorithm, HashResult, MultiAlgorithmHasher, + auth::{self, AuthError, MAX_EXECUTABLE_FILE_SIZE}, +}; +use futures::stream::{self, StreamExt}; +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use tracing::{debug, info, instrument, warn}; + +// ───────────────────────────────────────────────────────────────────────────── +// KernelResolvedExe newtype +// ───────────────────────────────────────────────────────────────────────────── + +/// A filesystem path resolved by the kernel, not by user input. +/// +/// On Linux this is `/proc/\[pid\]/exe`; on macOS it is `PROC_PIDPATHINFO`. +/// The private constructor ensures that only procmond's process enumeration +/// code (which reads sysinfo's `Process::exe()`) can construct this type. +/// +/// This prevents argv\[0\], cwd-relative paths, or root-relative paths from +/// ever reaching [`authorize_kernel_path`]. +#[derive(Debug, Clone)] +pub struct KernelResolvedExe(PathBuf); + +impl KernelResolvedExe { + /// Try to construct from sysinfo's `Process::exe()` output. + /// + /// The input **must** be an absolute, kernel-resolved path. In release + /// builds this is enforced by a real runtime check, not a + /// `debug_assert!` — any non-absolute path is rejected so malformed + /// `ProcessEvent.executable_path` values cannot become cwd-relative + /// hash targets inside the privileged collector. + /// + /// # Errors + /// + /// Returns `None` if `path` is not absolute. + #[must_use] + pub(crate) fn try_from_sysinfo_exe(path: PathBuf) -> Option { + path.is_absolute().then_some(Self(path)) + } + + /// Borrow the inner path. + #[must_use] + pub fn as_path(&self) -> &Path { + &self.0 + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// Authorization +// ───────────────────────────────────────────────────────────────────────────── + +/// Authorize a kernel-resolved executable path and return an **already- +/// opened** file descriptor for TOCTOU-safe hashing. +/// +/// Runs the shared predicates from [`daemoneye_lib::integrity::auth`]: +/// 1. Path length ≤ `MAX_EXECUTABLE_PATH_LEN` bytes. +/// 2. No `..` traversal components. +/// 3. Opens the file with `O_NOFOLLOW` (Unix) so the kernel rejects +/// symlink targets atomically. +/// 4. Fetches metadata from the opened fd (`fstat`), not the path. +/// 5. File is a regular file (via handle metadata). +/// 6. File size ≤ [`MAX_EXECUTABLE_FILE_SIZE`]. +/// +/// The caller **must** pass the returned `File` directly to +/// [`daemoneye_lib::integrity::MultiAlgorithmHasher::compute_from_file`] +/// — never re-open the path, or the TOCTOU defense is lost. +/// +/// # Errors +/// +/// Returns [`AuthError`] if any predicate fails or the open/fstat +/// itself errors. +pub fn authorize_kernel_path( + exe: &KernelResolvedExe, +) -> Result<(std::fs::File, std::fs::Metadata), AuthError> { + let path = exe.as_path(); + + auth::check_path_length(path)?; + auth::check_no_traversal(path)?; + + // On Unix, open with O_NOFOLLOW so the kernel refuses to open a + // symlink final-component atomically. On other platforms, fall back + // to checking `symlink_metadata` first (a narrow window, but + // platform parity would require a separate `OpenOptionsExt` + // implementation). + #[cfg(unix)] + let file = { + use std::os::unix::fs::OpenOptionsExt; + std::fs::OpenOptions::new() + .read(true) + // libc::O_NOFOLLOW — refuse to follow a symlink on the final + // path component. If `exe` itself is a symlink, open fails + // with `ELOOP` which we map to `SymlinkRejected` below. + .custom_flags(libc::O_NOFOLLOW) + .open(path) + .map_err(|source| { + if source.raw_os_error() == Some(libc::ELOOP) { + AuthError::SymlinkRejected { + path: path.to_path_buf(), + } + } else { + AuthError::Io { + path: path.to_path_buf(), + source, + } + } + })? + }; + #[cfg(not(unix))] + let file = { + // On non-Unix (Windows in particular), `File::open` on a + // directory returns `PermissionDenied` (CreateFile without + // FILE_FLAG_BACKUP_SEMANTICS), which would mask the + // type-of-target classification. Pre-stat with + // `symlink_metadata` so we can: + // 1. reject symlinks atomically (relative to the next + // open call) while still inside this function, + // 2. surface non-regular-file rejections as + // `AuthError::NotRegularFile` for cross-platform parity + // with the Unix `O_NOFOLLOW` path, + // before any open attempt. + // + // There is a narrow TOCTOU window between the stat and the + // open here that we accept for platform parity; security- + // critical Windows deployments should defer to the cap-std + // path in `BinaryHasherCollector` (which uses Dir handles + // opened before privilege drop) for forensic guarantees. + let pre_meta = std::fs::symlink_metadata(path).map_err(|source| AuthError::Io { + path: path.to_path_buf(), + source, + })?; + if pre_meta.file_type().is_symlink() { + return Err(AuthError::SymlinkRejected { + path: path.to_path_buf(), + }); + } + if !pre_meta.is_file() { + return Err(AuthError::NotRegularFile { + path: path.to_path_buf(), + }); + } + std::fs::File::open(path).map_err(|source| AuthError::Io { + path: path.to_path_buf(), + source, + })? + }; + + // Fetch metadata from the opened fd (fstat). This is the inode we + // authorized — not a path that may have been swapped. + let metadata = file.metadata().map_err(|source| AuthError::Io { + path: path.to_path_buf(), + source, + })?; + + auth::check_regular_file(path, &metadata)?; + auth::check_size(&metadata, MAX_EXECUTABLE_FILE_SIZE)?; + + Ok((file, metadata)) +} + +// ───────────────────────────────────────────────────────────────────────────── +// HashPassStats +// ───────────────────────────────────────────────────────────────────────────── + +/// Aggregate statistics for a post-enumeration hash-population pass. +/// +/// Extends the original `process_collector::HashCoverageStats` with +/// auth-failure and nonauthoritative counters for telemetry. +#[derive(Debug, Clone, Copy, Default)] +pub struct HashPassStats { + /// Number of unique executable paths seen across the scan. + pub(crate) unique_paths: usize, + /// Number of unique paths that were successfully hashed. + pub(crate) hashed: usize, + /// Number of unique paths that failed authorization. + pub(crate) auth_failures: usize, + /// Number of paths where the engine returned `Nonauthoritative`. + pub(crate) nonauthoritative: usize, + /// Number of unique paths where hashing failed (I/O, timeout, etc.). + pub(crate) io_failures: usize, +} + +impl HashPassStats { + /// Total number of unique paths processed (successful and unsuccessful). + #[must_use] + pub const fn total_processed(&self) -> usize { + self.hashed + .saturating_add(self.auth_failures) + .saturating_add(self.nonauthoritative) + .saturating_add(self.io_failures) + } +} + +// ───────────────────────────────────────────────────────────────────────────── +// populate_hashes +// ───────────────────────────────────────────────────────────────────────────── + +/// Parallel, authorization-gated hash pass for process events. +/// +/// Deduplicates by `executable_path`, runs [`authorize_kernel_path`] on +/// each unique path, then hashes authorized paths concurrently using +/// `buffer_unordered(engine.max_concurrent())`. +/// +/// This replaces `process_collector::populate_executable_hashes` with: +/// - Authorization before every hash (todo #011). +/// - `buffer_unordered` parallelism (todo #010). +/// +/// Errors for individual files are logged and counted but never propagated. +#[instrument(skip_all, fields(event_count = events.len()))] +pub async fn populate_hashes( + events: &mut [ProcessEvent], + hasher: &Arc, +) -> HashPassStats { + let mut stats = HashPassStats::default(); + + // Phase 1: Dedup by executable_path AND build a path -> event_indices + // map. The map is what makes Phase 2 cancellation-safe: when each + // hash completes, we stamp every event index that shares that path + // DIRECTLY on the `&mut [ProcessEvent]` slice owned by the caller. + // Because the slice lives above us, those mutations survive even if + // our future is dropped by an outer `tokio::time::timeout`. + // + // Phase 1 also resets stale hash state on every event up front so + // even if the caller cancels us before any hash completes, reused + // events never carry hashes from a prior scan. + let mut path_to_indices: HashMap> = HashMap::new(); + for (idx, event) in events.iter_mut().enumerate() { + event.executable_hash = None; + event.hash_algorithm = None; + if let Some(ref raw) = event.executable_path { + path_to_indices.entry(raw.clone()).or_default().push(idx); + } + } + stats.unique_paths = path_to_indices.len(); + + if path_to_indices.is_empty() { + return stats; + } + + // Phase 2: Authorize + hash in parallel. Results are stamped onto + // events as each hash finishes — NOT buffered into a Vec and + // processed in a separate phase. If a caller wraps this future in + // `tokio::time::timeout` (or cancels via select!), the loop body + // between two `next().await` yield points runs synchronously and + // commits its outcome to `events` before the next opportunity to + // be cancelled. Cancellation at a yield point loses ONLY the + // in-flight hashes (at most `max_concurrent`); every event whose + // hash completed before the yield is already stamped. + // + // The `local_stats` tally is also updated synchronously alongside + // the event stamping so the returned counters never disagree with + // the visible state of `events`. On cancel the function never + // returns and the caller cannot read these stats — but they CAN + // count `executable_hash.is_some()` on `events` to recover the + // partial-coverage figure. + let concurrency = hasher.max_concurrent(); + let engine = Arc::clone(hasher); + + // Materialize the work list up front so the stream does not hold + // a borrow on `path_to_indices` across the loop body that mutates + // `events` (which itself doesn't borrow `path_to_indices`, but + // rust-analyzer is conservative about disjoint borrows). + let work: Vec = path_to_indices.keys().cloned().collect(); + let mut hash_stream = stream::iter(work) + .map(|raw| { + let h = Arc::clone(&engine); + async move { + let outcome = if let Some(exe) = + KernelResolvedExe::try_from_sysinfo_exe(PathBuf::from(&raw)) + { + hash_one(&exe, &h).await + } else { + debug!( + path = ?raw, + "rejecting non-absolute path before hashing" + ); + HashOutcome::AuthFailed + }; + (raw, outcome) + } + }) + .buffer_unordered(concurrency); + + while let Some((raw, outcome)) = hash_stream.next().await { + match outcome { + HashOutcome::Hashed { hex, algorithm } => { + stats.hashed = stats.hashed.saturating_add(1); + // Stamp every event sharing this path. Direct &mut on + // `events` — survives drop because the caller owns it. + if let Some(indices) = path_to_indices.get(&raw) { + for &idx in indices { + if let Some(event) = events.get_mut(idx) { + event.executable_hash = Some(hex.clone()); + event.hash_algorithm = Some(algorithm.clone()); + } + } + } + } + HashOutcome::AuthFailed => { + stats.auth_failures = stats.auth_failures.saturating_add(1); + } + HashOutcome::Nonauthoritative => { + stats.nonauthoritative = stats.nonauthoritative.saturating_add(1); + } + HashOutcome::IoFailure => { + stats.io_failures = stats.io_failures.saturating_add(1); + } + } + } + drop(hash_stream); + + // Telemetry: emit coverage stats so operators can distinguish + // "feature disabled" from "files inaccessible". + info!( + unique_paths = stats.unique_paths, + hashed = stats.hashed, + auth_failures = stats.auth_failures, + nonauthoritative = stats.nonauthoritative, + io_failures = stats.io_failures, + "hash pass completed" + ); + + debug_assert_eq!( + stats.total_processed(), + stats.unique_paths, + "total_processed ({}) != unique_paths ({}): every unique path must have an outcome", + stats.total_processed(), + stats.unique_paths, + ); + + stats +} + +/// Outcome of a single hash attempt. +enum HashOutcome { + /// Successfully hashed — contains the hex digest and algorithm name. + Hashed { hex: String, algorithm: String }, + /// Authorization rejected the path. + AuthFailed, + /// Engine detected mid-read mutation. + Nonauthoritative, + /// I/O or other engine error. + IoFailure, +} + +/// Authorize and hash a single executable. +/// +/// Uses the TOCTOU-safe flow: `authorize_kernel_path` returns an +/// already-opened `File` (with `O_NOFOLLOW` on Unix), and we hand that +/// file descriptor directly to +/// [`MultiAlgorithmHasher::compute_from_file`]. Re-opening the path +/// between authorization and hashing would reintroduce the TOCTOU +/// window cap-std was added to close. +#[allow(clippy::pattern_type_mismatch)] +async fn hash_one(exe: &KernelResolvedExe, hasher: &MultiAlgorithmHasher) -> HashOutcome { + // Authorization gate — returns an opened file handle bound to the + // exact inode the predicates were evaluated against. + let (file, _meta) = match authorize_kernel_path(exe) { + Ok(pair) => pair, + Err(ref err) => { + let display_path = auth::bytes_safe_display(exe.as_path(), 200); + #[allow(clippy::wildcard_enum_match_arm)] + match err { + AuthError::Io { source, .. } + if source.kind() == std::io::ErrorKind::PermissionDenied => + { + debug!(path = %display_path, error = %err, "hash auth skipped: permission denied"); + } + AuthError::Io { source, .. } if source.kind() == std::io::ErrorKind::NotFound => { + debug!(path = %display_path, error = %err, "hash auth skipped: file not found"); + } + _ => { + warn!(path = %display_path, error = %err, "hash auth rejected"); + } + } + return HashOutcome::AuthFailed; + } + }; + + // Hash the authorized file descriptor. NEVER reopen by path. + match hasher.compute_from_file(exe.as_path(), file).await { + Ok(result) => { + if let Some(hex) = primary_hash_hex(&result) { + HashOutcome::Hashed { + hex, + algorithm: HashAlgorithm::Sha256.wire_name().to_owned(), + } + } else { + let available: Vec<_> = result.hashes.keys().collect(); + warn!( + path = ?exe.as_path(), + ?available, + "hash result missing SHA-256; available algorithms listed" + ); + HashOutcome::IoFailure + } + } + Err(daemoneye_lib::integrity::HashError::Nonauthoritative { .. }) => { + debug!(path = ?exe.as_path(), "hash: file mutated mid-read (nonauthoritative)"); + HashOutcome::Nonauthoritative + } + Err(daemoneye_lib::integrity::HashError::PermissionDenied { .. }) => { + debug!(path = ?exe.as_path(), "hash skipped: permission denied"); + HashOutcome::IoFailure + } + #[allow(clippy::wildcard_enum_match_arm)] + Err(ref err) => { + warn!(path = ?exe.as_path(), error = %err, "hash failed"); + HashOutcome::IoFailure + } + } +} + +/// Extract the primary (SHA-256) hex string from a [`HashResult`]. +fn primary_hash_hex(result: &HashResult) -> Option { + result.sha256().map(str::to_owned) +} + +// ───────────────────────────────────────────────────────────────────────────── +// Tests +// ───────────────────────────────────────────────────────────────────────────── + +#[cfg(test)] +#[allow( + clippy::expect_used, + clippy::unwrap_used, + clippy::panic, + clippy::uninlined_format_args, + clippy::string_add +)] +mod tests { + use super::*; + use daemoneye_lib::integrity::HasherConfig; + use std::fs; + use tempfile::NamedTempFile; + + // Only needed by the `#[cfg(unix)]` path-length boundary tests below. + #[cfg(unix)] + use daemoneye_lib::integrity::auth::MAX_EXECUTABLE_PATH_LEN; + + fn new_event(pid: u32, exe: &str) -> ProcessEvent { + ProcessEvent { + pid, + ppid: None, + name: format!("proc-{pid}"), + executable_path: Some(exe.to_owned()), + command_line: Vec::new(), + start_time: None, + cpu_usage: None, + memory_usage: None, + executable_hash: None, + hash_algorithm: None, + user_id: None, + accessible: true, + file_exists: true, + timestamp: std::time::SystemTime::now(), + platform_metadata: None, + } + } + + // ── authorize_kernel_path ───────────────────────────────────────── + // + // The following tests construct `KernelResolvedExe` from hardcoded + // Unix-style paths (`/usr/bin/...`, `/definitely/does/not/exist/...`). + // On Windows, `Path::is_absolute()` returns `false` for these because + // Windows requires a drive letter (`C:\...`) or UNC prefix, so the + // `debug_assert!(path.is_absolute())` inside + // `KernelResolvedExe::from_sysinfo_exe` would panic at test time. + // + // Gating with `#[cfg(unix)]` keeps the tests honest about what they + // exercise (Unix path-handling semantics) without weakening the + // production invariant on Windows, where sysinfo would yield + // `C:\Windows\System32\...`-style paths that are genuinely absolute. + + #[cfg(unix)] + #[test] + fn auth_rejects_path_too_long() { + let long_path = PathBuf::from("/".to_owned() + &"a".repeat(MAX_EXECUTABLE_PATH_LEN + 1)); + let exe = KernelResolvedExe::try_from_sysinfo_exe(long_path).expect("absolute"); + assert!(matches!( + authorize_kernel_path(&exe), + Err(AuthError::PathTooLong { .. }) + )); + } + + #[cfg(unix)] + #[test] + fn auth_rejects_traversal() { + let exe = KernelResolvedExe::try_from_sysinfo_exe(PathBuf::from("/usr/bin/../sbin/evil")) + .expect("absolute"); + assert!(matches!( + authorize_kernel_path(&exe), + Err(AuthError::PathTraversal { .. }) + )); + } + + #[cfg(unix)] + #[test] + fn auth_rejects_nonexistent() { + let exe = KernelResolvedExe::try_from_sysinfo_exe(PathBuf::from( + "/definitely/does/not/exist/xyz", + )) + .expect("absolute"); + assert!(matches!( + authorize_kernel_path(&exe), + Err(AuthError::Io { .. }) + )); + } + + #[test] + fn auth_rejects_directory() { + let dir = tempfile::tempdir().unwrap(); + let exe = + KernelResolvedExe::try_from_sysinfo_exe(dir.path().to_path_buf()).expect("absolute"); + assert!(matches!( + authorize_kernel_path(&exe), + Err(AuthError::NotRegularFile { .. }) + )); + } + + #[test] + fn auth_accepts_regular_file() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"test binary content").unwrap(); + let exe = + KernelResolvedExe::try_from_sysinfo_exe(tmp.path().to_path_buf()).expect("absolute"); + assert!(authorize_kernel_path(&exe).is_ok()); + } + + #[cfg(unix)] + #[test] + fn auth_rejects_symlink() { + let dir = tempfile::tempdir().unwrap(); + let target = dir.path().join("real_file"); + let link = dir.path().join("symlink_to_real"); + fs::write(&target, b"real binary content").unwrap(); + std::os::unix::fs::symlink(&target, &link).unwrap(); + let exe = KernelResolvedExe::try_from_sysinfo_exe(link).expect("absolute"); + assert!(matches!( + authorize_kernel_path(&exe), + Err(AuthError::SymlinkRejected { .. }) + )); + } + + #[cfg(unix)] + #[test] + fn auth_boundary_4096_bytes() { + // Path of exactly 4096 bytes should pass length check (may fail + // on other checks like file-not-found, which is fine). + let path = PathBuf::from("/".to_owned() + &"a".repeat(MAX_EXECUTABLE_PATH_LEN - 1)); + assert_eq!(path.as_os_str().len(), MAX_EXECUTABLE_PATH_LEN); + let exe = KernelResolvedExe::try_from_sysinfo_exe(path).expect("absolute"); + let result = authorize_kernel_path(&exe); + // Should NOT be PathTooLong — it may be FileNotFound, which is fine. + assert!(!matches!(result, Err(AuthError::PathTooLong { .. }))); + } + + #[cfg(unix)] + #[test] + fn auth_boundary_multi_byte_utf8() { + // 4-byte emoji repeated to cross the boundary. Must not panic. + // Prefix with "/" so the path is absolute (required by debug_assert in + // KernelResolvedExe::from_sysinfo_exe). "/" + 1025 × 4 bytes = 4101 bytes + // which still exceeds MAX_EXECUTABLE_PATH_LEN (4096). + let emoji_path = PathBuf::from("/".to_owned() + &"\u{1F600}".repeat(1025)); + let exe = KernelResolvedExe::try_from_sysinfo_exe(emoji_path).expect("absolute"); + let result = authorize_kernel_path(&exe); + assert!(matches!(result, Err(AuthError::PathTooLong { .. }))); + } + + // ── populate_hashes ────────────────────────────────────────────── + + #[tokio::test] + async fn populate_hashes_fills_hash_and_algorithm() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"parallel hash pass").unwrap(); + let path = tmp.path().to_string_lossy().into_owned(); + + let mut events = vec![new_event(1, &path), new_event(2, &path)]; + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 1); + assert_eq!(stats.hashed, 1); + assert_eq!(stats.auth_failures, 0); + + for event in &events { + assert_eq!(event.hash_algorithm.as_deref(), Some("sha256")); + assert!( + event + .executable_hash + .as_deref() + .is_some_and(|h| h.len() == 64) + ); + } + } + + #[tokio::test] + async fn populate_hashes_dedup_works() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"dedup test").unwrap(); + let path = tmp.path().to_string_lossy().into_owned(); + + let mut events: Vec = (0..50_u32).map(|pid| new_event(pid, &path)).collect(); + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 1); + assert_eq!(stats.hashed, 1); + assert!(events.iter().all(|e| e.executable_hash.is_some())); + } + + // Uses Unix-style nonexistent paths that are NOT absolute on Windows, + // so `KernelResolvedExe::from_sysinfo_exe`'s `debug_assert!(is_absolute)` + // would fire at test time. The populate_hashes logic is platform-neutral; + // the production invariant is what differs. + #[cfg(unix)] + #[tokio::test] + async fn populate_hashes_missing_file_is_nonfatal() { + let mut events = vec![ + new_event(1, "/definitely/does/not/exist/xyz"), + new_event(2, "/also/not/here"), + ]; + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 2); + assert_eq!(stats.hashed, 0); + // Missing files fail at auth (I/O not found). + assert_eq!(stats.auth_failures, 2); + for event in &events { + assert!(event.executable_hash.is_none()); + } + } + + #[tokio::test] + async fn populate_hashes_empty_slice_is_noop() { + let mut events: Vec = Vec::new(); + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 0); + assert_eq!(stats.hashed, 0); + } + + #[tokio::test] + async fn populate_hashes_skips_events_without_path() { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), b"with path").unwrap(); + // Persist the file on disk without leaking the fd. `.keep()` + // converts the temp file into a regular file at the same path + // and hands us back a `TempPath` we can drop safely. + let (_file, path_guard) = tmp.keep().unwrap(); + let path = path_guard.to_string_lossy().into_owned(); + + let mut event_without_path = new_event(2, "ignored"); + event_without_path.executable_path = None; + + let mut events = vec![new_event(1, &path), event_without_path]; + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 1); + assert!(events.first().is_some_and(|e| e.executable_hash.is_some())); + assert!(events.get(1).is_some_and(|e| e.executable_hash.is_none())); + } + + // Mixes real (platform-neutral) temp files with a hardcoded Unix-style + // nonexistent path. Gated for the same reason as the other tests above. + #[cfg(unix)] + #[tokio::test] + async fn populate_hashes_mixed_success_and_failure() { + // Two real files that will hash successfully, one nonexistent path + // that will fail auth (file not found → AuthFailed). + let tmp1 = NamedTempFile::new().unwrap(); + fs::write(tmp1.path(), b"real binary one").unwrap(); + let tmp2 = NamedTempFile::new().unwrap(); + fs::write(tmp2.path(), b"real binary two").unwrap(); + + let path1 = tmp1.path().to_string_lossy().into_owned(); + let path2 = tmp2.path().to_string_lossy().into_owned(); + + let mut events = vec![ + new_event(1, &path1), + new_event(2, &path2), + new_event(3, "/definitely/does/not/exist/mixed-test"), + ]; + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 3); + assert_eq!(stats.hashed, 2, "expected 2 successful hashes"); + assert_eq!( + stats.auth_failures, 1, + "expected 1 auth failure for nonexistent path" + ); + } + + // Uses a hardcoded Unix-style nonexistent path. Gated for the same + // reason as the other tests above. + #[cfg(unix)] + #[tokio::test] + async fn populate_hashes_clears_stale_hashes_from_reused_events() { + // Simulate a reused ProcessEvent carrying hash state from a prior + // scan. If this scan cannot authorize the file (here: nonexistent + // path), populate_hashes MUST clear the stale values rather than + // leave them in place. + let mut event = new_event(1, "/definitely/does/not/exist/stale-test"); + // Seed with sentinel values that are obviously not produced by the + // real engine. The intent is to prove populate_hashes CLEARS any + // pre-existing state, not to test a particular algorithm label. + event.executable_hash = Some("stale-digest".to_owned()); + event.hash_algorithm = Some("stale-algo".to_owned()); + + let mut events = vec![event]; + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + let stats = populate_hashes(&mut events, &hasher).await; + + assert_eq!(stats.unique_paths, 1); + assert_eq!(stats.hashed, 0); + assert_eq!(stats.auth_failures, 1); + let first = events.first().unwrap(); + assert!( + first.executable_hash.is_none(), + "stale executable_hash must be cleared" + ); + assert!( + first.hash_algorithm.is_none(), + "stale hash_algorithm must be cleared" + ); + } + + #[tokio::test] + async fn populate_hashes_parallel_multiple_files() { + // Create multiple temp files to exercise buffer_unordered concurrency. + // Use `.keep()` to persist files without leaking fds. + let kept: Vec<_> = (0..10) + .map(|i| { + let tmp = NamedTempFile::new().unwrap(); + fs::write(tmp.path(), format!("binary-{i}")).unwrap(); + tmp.keep().unwrap() + }) + .collect(); + let files: Vec = kept + .iter() + .map(|tuple| { + let (_, ref p) = *tuple; + p.to_string_lossy().into_owned() + }) + .collect(); + + let mut events: Vec = files + .iter() + .enumerate() + .map(|(i, path)| { + #[allow(clippy::as_conversions)] + new_event(i as u32, path) + }) + .collect(); + + let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default()).unwrap()); + let stats = populate_hashes(&mut events, &hasher).await; + assert_eq!(stats.unique_paths, 10); + assert_eq!(stats.hashed, 10); + assert!(events.iter().all(|e| e.executable_hash.is_some())); + } +} diff --git a/procmond/src/lib.rs b/procmond/src/lib.rs index 40afdaf0..8efded08 100644 --- a/procmond/src/lib.rs +++ b/procmond/src/lib.rs @@ -3,6 +3,7 @@ pub mod event_bus_connector; pub mod event_source; +pub(crate) mod hash_pass; pub mod lifecycle; pub mod monitor_collector; pub mod process_collector; @@ -142,6 +143,22 @@ pub struct ProcessMessageHandler { pub database: Arc>, /// Process collector implementation for platform-agnostic process enumeration pub collector: Box, + /// Optional shared hash engine. This field is the **authoritative** + /// enable flag for executable hashing on this handler: if `Some`, + /// enumeration runs a post-pass via `hash_pass::populate_hashes` to + /// fill `executable_hash` / `hash_algorithm` on every event; if + /// `None`, those fields always stay `None`. + /// + /// The composition root (`procmond/src/main.rs`) constructs the + /// engine only when `--compute-hashes == true` is set on the CLI, + /// and threads the same `Arc` into every holder (`ProcessEventSource`, + /// `ProcmondMonitorCollector`, and this handler). Programmatic + /// callers MUST follow the same contract: pass `Some(engine)` IFF + /// hashing is actually desired. The handler does not cross-check + /// the underlying collector's `ProcessCollectionConfig` — `hasher` + /// is the single source of truth so a mismatch cannot degrade + /// into the silent no-op resolved in P1 #013. + pub hasher: Option>, } impl ProcessMessageHandler { @@ -174,6 +191,7 @@ impl ProcessMessageHandler { Self { database, collector, + hasher: None, } } @@ -210,9 +228,37 @@ impl ProcessMessageHandler { Self { database, collector, + hasher: None, } } + /// Attach a shared hash engine. When set, enumeration results are + /// post-processed by + /// `hash_pass::populate_hashes` to fill + /// `executable_hash` and `hash_algorithm` on every `ProcessEvent`. + /// + /// Pass `None` to disable hashing. The typical construction flow is: + /// + /// ```rust,no_run + /// use procmond::ProcessMessageHandler; + /// use daemoneye_lib::integrity::{HasherConfig, MultiAlgorithmHasher}; + /// use std::sync::Arc; + /// + /// # fn example(mut handler: ProcessMessageHandler) -> Result<(), Box> { + /// let hasher = Arc::new(MultiAlgorithmHasher::new(HasherConfig::default())?); + /// handler = handler.with_hasher(Some(hasher)); + /// # Ok(()) + /// # } + /// ``` + #[must_use] + pub fn with_hasher( + mut self, + hasher: Option>, + ) -> Self { + self.hasher = hasher; + self + } + pub async fn handle_detection_task( &self, task: DetectionTask, @@ -267,7 +313,8 @@ impl ProcessMessageHandler { &self, task: &DetectionTask, ) -> Result { - use tracing::{debug, error}; + use std::time::Duration; + use tracing::{debug, error, warn}; debug!( task_id = %task.task_id, @@ -279,7 +326,53 @@ impl ProcessMessageHandler { let enumeration_result = self.collector.collect_processes().await; match enumeration_result { - Ok((process_events, collection_stats)) => { + Ok((mut process_events, collection_stats)) => { + // Post-enumeration hash population pass. This runs + // exactly once per scan and dedupes by executable path + // before hashing, so the amortized cost across 10k + // processes is typically 200-500 hashes (the number of + // unique executables on a real host). The engine's + // shared quick_cache then makes subsequent scans + // ~zero-cost in steady state. + // + // Request-scoped timeout: the engine enforces a + // `timeout_per_file` on each hash, but 10k unique files + // × multi-second stalls on slow/hostile storage could + // still wedge the privileged collector indefinitely. + // Wrap the whole pass in an overall deadline so an + // `EnumerateProcesses` request always has a hard upper + // bound. On timeout we keep the events that were + // already stamped (partial coverage is fine — downstream + // handles missing hashes) and log the truncation. + if let Some(hasher) = self.hasher.as_ref() { + const HASH_PASS_OVERALL_DEADLINE: Duration = Duration::from_secs(60); + match tokio::time::timeout( + HASH_PASS_OVERALL_DEADLINE, + hash_pass::populate_hashes(&mut process_events, hasher), + ) + .await + { + Ok(stats) => { + debug!( + task_id = %task.task_id, + unique_paths = stats.unique_paths, + hashed = stats.hashed, + auth_failures = stats.auth_failures, + io_failures = stats.io_failures, + nonauthoritative = stats.nonauthoritative, + "hash population completed" + ); + } + Err(_elapsed) => { + warn!( + task_id = %task.task_id, + deadline_secs = HASH_PASS_OVERALL_DEADLINE.as_secs(), + "hash population exceeded request deadline; partial coverage recorded" + ); + } + } + } + debug!( task_id = %task.task_id, total_found = collection_stats.total_processes, @@ -362,6 +455,7 @@ impl ProcessMessageHandler { /// cpu_usage: Some(5.0), /// memory_usage: Some(1024 * 1024), /// executable_hash: None, + /// hash_algorithm: None, /// user_id: Some("1000".to_string()), /// accessible: true, /// file_exists: true, @@ -420,8 +514,18 @@ impl ProcessMessageHandler { 0 }; - // Check if executable hash exists before moving the value - let has_executable_hash = event.executable_hash.is_some(); + // Propagate the hash_algorithm carried on the event unchanged. + // Invariant (enforced by `hash_pass::populate_hashes` and every + // `ProcessEvent` constructor): `executable_hash.is_some() == + // hash_algorithm.is_some()`. We do NOT reinstate a "sha256" + // default here — doing so would mislabel non-SHA256 results + // (e.g. BLAKE3) on the wire, which is worse than an absent + // algorithm field. + debug_assert_eq!( + event.executable_hash.is_some(), + event.hash_algorithm.is_some(), + "ProcessEvent invariant violated: executable_hash and hash_algorithm must both be Some or both None" + ); ProtoProcessRecord { pid: event.pid, @@ -433,7 +537,7 @@ impl ProcessMessageHandler { cpu_usage: event.cpu_usage, memory_usage: event.memory_usage, executable_hash: event.executable_hash, - hash_algorithm: has_executable_hash.then(|| "sha256".to_owned()), + hash_algorithm: event.hash_algorithm, user_id: event.user_id, accessible: event.accessible, file_exists: event.file_exists, @@ -550,6 +654,7 @@ mod tests { cpu_usage: Some(0.1), memory_usage: Some(1024 * 1024), executable_hash: Some("hash1".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("0".to_string()), accessible: true, file_exists: true, @@ -566,6 +671,7 @@ mod tests { cpu_usage: Some(5.0), memory_usage: Some(2048 * 1024), executable_hash: Some("hash2".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -582,6 +688,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: false, file_exists: false, @@ -825,6 +932,7 @@ mod tests { cpu_usage: Some(5.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -992,6 +1100,7 @@ mod tests { cpu_usage: Some(1.0), memory_usage: Some(1024), executable_hash: None, + hash_algorithm: None, user_id: Some("0".to_string()), accessible: true, file_exists: true, @@ -1008,6 +1117,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: false, file_exists: false, @@ -1127,6 +1237,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: false, file_exists: false, @@ -1161,6 +1272,7 @@ mod tests { cpu_usage: Some(2.5), memory_usage: Some(4096), executable_hash: Some("abcdef123456".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1001".to_string()), accessible: true, file_exists: true, diff --git a/procmond/src/lifecycle.rs b/procmond/src/lifecycle.rs index 1c1577e4..bb37c5d3 100644 --- a/procmond/src/lifecycle.rs +++ b/procmond/src/lifecycle.rs @@ -127,9 +127,18 @@ pub struct ProcessSnapshot { /// Memory usage in bytes at snapshot time pub memory_usage: Option, - /// SHA-256 hash of executable + /// Hex-encoded cryptographic hash of the executable. See + /// [`collector_core::ProcessEvent::executable_hash`] for details. pub executable_hash: Option, + /// Canonical lowercase name of the algorithm that produced + /// [`Self::executable_hash`] (e.g. `"sha256"`). Always `None` when + /// `executable_hash` is `None`; always `Some` when it is populated. + /// The lifecycle diff at `ProcessLifecycleTracker::diff_snapshots` + /// compares `(executable_hash, hash_algorithm)` as a tuple so that + /// switching the canonical algorithm is not a silent breaking change. + pub hash_algorithm: Option, + /// User ID running the process pub user_id: Option, @@ -158,6 +167,7 @@ impl From for ProcessSnapshot { cpu_usage: event.cpu_usage, memory_usage: event.memory_usage, executable_hash: event.executable_hash, + hash_algorithm: event.hash_algorithm, user_id: event.user_id, accessible: event.accessible, file_exists: event.file_exists, @@ -179,6 +189,7 @@ impl From for ProcessEvent { cpu_usage: snapshot.cpu_usage, memory_usage: snapshot.memory_usage, executable_hash: snapshot.executable_hash, + hash_algorithm: snapshot.hash_algorithm, user_id: snapshot.user_id, accessible: snapshot.accessible, file_exists: snapshot.file_exists, @@ -628,8 +639,22 @@ impl ProcessLifecycleTracker { } } - // Check executable hash changes - if previous.executable_hash != current.executable_hash + // Check executable hash changes. Compare (hash, algorithm) as a + // TUPLE so switching the canonical algorithm across procmond + // versions is not a silent breaking change — if algo changes but + // hex doesn't, we still report the drift and let the audit + // pipeline decide how to interpret it. The is_some() guards + // prevent false-positive diffs when hashing transitions from + // disabled to enabled across scan cycles. + let prev_pair = ( + previous.executable_hash.as_deref(), + previous.hash_algorithm.as_deref(), + ); + let curr_pair = ( + current.executable_hash.as_deref(), + current.hash_algorithm.as_deref(), + ); + if prev_pair != curr_pair && previous.executable_hash.is_some() && current.executable_hash.is_some() { @@ -818,6 +843,7 @@ mod tests { cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/procmond/src/linux_collector.rs b/procmond/src/linux_collector.rs index 1620a37c..e31f9840 100644 --- a/procmond/src/linux_collector.rs +++ b/procmond/src/linux_collector.rs @@ -642,9 +642,12 @@ impl LinuxProcessCollector { .and_then(|s| s.parse::().ok()) .and_then(|jiffies| self.calculate_start_time(jiffies)); - // Compute executable hash if requested - // TODO: Implement executable hashing (issue #40) + // Executable hash populated in a post-enumeration pass (see + // `hash_pass::populate_hashes`) so that the synchronous + // per-process conversion path stays off the async runtime. + // Invariant: `executable_hash.is_some() == hash_algorithm.is_some()`. let executable_hash: Option = None; + let hash_algorithm: Option = None; // Serialize enhanced metadata for platform_metadata field let platform_metadata = if self.base_config.collect_enhanced_metadata { @@ -672,6 +675,7 @@ impl LinuxProcessCollector { cpu_usage, memory_usage, executable_hash, + hash_algorithm, user_id, accessible, file_exists, @@ -826,6 +830,18 @@ impl ProcessCollector for LinuxProcessCollector { Ok((events, stats)) } + /// Collect a single process by PID. + /// + /// **Note**: this API does NOT run the post-enumeration hash pass, + /// so the returned `ProcessEvent` always has `executable_hash = None` + /// / `hash_algorithm = None` regardless of + /// [`ProcessCollectionConfig::compute_executable_hashes`]. The hash + /// pass is a batch operation (it dedupes across all collected + /// events) and runs in the multi-process path only. Callers that + /// need the hash for a single PID should invoke + /// `crate::hash_pass::populate_hashes` explicitly with a slice + /// containing the returned event plus an injected + /// `Arc`. async fn collect_process(&self, pid: u32) -> ProcessCollectionResult { debug!( collector = self.name(), @@ -947,9 +963,11 @@ impl LinuxProcessCollector { (None, None, None) }; - // Compute executable hash if requested - // TODO: Implement executable hashing (issue #40) + // Executable hash populated in a post-enumeration pass (see + // `hash_pass::populate_hashes`). Invariant: + // `executable_hash.is_some() == hash_algorithm.is_some()`. let executable_hash: Option = None; + let hash_algorithm: Option = None; let user_id = process.user_id().map(|uid| uid.to_string()); let accessible = true; @@ -984,6 +1002,7 @@ impl LinuxProcessCollector { cpu_usage, memory_usage, executable_hash, + hash_algorithm, user_id, accessible, file_exists, diff --git a/procmond/src/macos_collector.rs b/procmond/src/macos_collector.rs index 293335be..d2e6f531 100644 --- a/procmond/src/macos_collector.rs +++ b/procmond/src/macos_collector.rs @@ -503,9 +503,10 @@ impl EnhancedMacOSCollector { None }; - // Compute executable hash if requested - // TODO: Implement executable hashing - compute SHA-256 hash of executable file + // Executable hash populated in a post-enumeration pass on the + // collector level. See `hash_pass::populate_hashes`. let executable_hash: Option = None; + let hash_algorithm: Option = None; let user_id = process.user_id().map(|u| u.to_string()); let accessible = true; // If we can read process info, it's accessible @@ -535,6 +536,7 @@ impl EnhancedMacOSCollector { cpu_usage, memory_usage, executable_hash, + hash_algorithm, user_id, accessible, file_exists, diff --git a/procmond/src/main.rs b/procmond/src/main.rs index f8e3059e..b3b5caeb 100644 --- a/procmond/src/main.rs +++ b/procmond/src/main.rs @@ -1,8 +1,13 @@ #![forbid(unsafe_code)] +use anyhow::Context as _; use clap::Parser; use collector_core::{CollectionEvent, Collector, CollectorConfig, CollectorRegistrationConfig}; -use daemoneye_lib::{config, storage, telemetry}; +use daemoneye_lib::{ + config, + integrity::{HashComputer, HasherConfig, MultiAlgorithmHasher}, + storage, telemetry, +}; use procmond::{ ProcessEventSource, ProcessSourceConfig, event_bus_connector::EventBusConnector, @@ -71,7 +76,7 @@ struct Cli { } #[tokio::main] -pub async fn main() -> Result<(), Box> { +pub async fn main() -> anyhow::Result<()> { // Parse CLI arguments first - this will handle --help and --version automatically let cli = Cli::parse(); @@ -122,6 +127,40 @@ pub async fn main() -> Result<(), Box> { "Database stats retrieved" ); + // ======================================================================== + // Composition root for the shared executable-hash engine. + // + // When `--compute-hashes` is enabled, construct exactly one + // `Arc` here and clone it into every holder + // (actor-mode `ProcmondMonitorCollector` and standalone-mode + // `ProcessEventSource`). Sharing the `Arc` guarantees a single + // policy (one concurrency cap, one algorithm list, one size + // budget) no matter which path the process runs through. See + // `daemoneye-lib::integrity::MultiAlgorithmHasher` rustdoc for the + // statelessness invariant that protects the shared `Arc` across + // trust domains. + // + // If engine construction fails when `--compute-hashes` is set, the + // error propagates immediately via `?` — there is no silent fallback. + // ======================================================================== + let shared_hasher: Option> = if cli.compute_hashes { + let engine = MultiAlgorithmHasher::new(HasherConfig::default()) + .context("failed to construct hash engine for --compute-hashes")?; + let algos: Vec<_> = engine + .supported_algorithms() + .iter() + .map(|a| a.wire_name()) + .collect::>(); + info!( + max_concurrent = engine.max_concurrent(), + algorithms = ?algos, + "procmond.hash.subsystem enabled=true" + ); + Some(Arc::new(engine)) + } else { + None + }; + // Check for broker configuration via environment variable // DAEMONEYE_BROKER_SOCKET: If set, use actor mode with EventBusConnector // If not set, use standalone mode with collector-core @@ -159,7 +198,8 @@ pub async fn main() -> Result<(), Box> { Arc::clone(&db_manager), monitor_config, message_receiver, - )?; + )? + .with_hasher(shared_hasher.as_ref().map(Arc::clone)); // Initialize EventBusConnector with WAL directory let wal_dir = PathBuf::from(&cli.database).parent().map_or_else( @@ -481,8 +521,9 @@ pub async fn main() -> Result<(), Box> { ..Default::default() }; - // Create process event source - let process_source = ProcessEventSource::with_config(db_manager, process_config); + // Create process event source with shared hasher injected. + let process_source = ProcessEventSource::with_config(db_manager, process_config) + .with_hasher(shared_hasher.as_ref().map(Arc::clone)); // Log RPC service status let registration_enabled = collector_config diff --git a/procmond/src/monitor_collector.rs b/procmond/src/monitor_collector.rs index bb2bd617..e744f798 100644 --- a/procmond/src/monitor_collector.rs +++ b/procmond/src/monitor_collector.rs @@ -6,6 +6,7 @@ use crate::{ event_bus_connector::{EventBusConnector, ProcessEventType}, + hash_pass::populate_hashes, lifecycle::{LifecycleTrackingConfig, ProcessLifecycleEvent, ProcessLifecycleTracker}, process_collector::{ProcessCollectionConfig, ProcessCollector, SysinfoProcessCollector}, }; @@ -16,7 +17,7 @@ use collector_core::{ MonitorCollector as MonitorCollectorTrait, MonitorCollectorConfig, MonitorCollectorStats, MonitorCollectorStatsSnapshot, SourceCaps, TriggerManager, }; -use daemoneye_lib::{storage, telemetry::PerformanceTimer}; +use daemoneye_lib::{integrity::MultiAlgorithmHasher, storage, telemetry::PerformanceTimer}; use std::{ sync::{ Arc, @@ -373,6 +374,17 @@ pub struct ProcmondMonitorCollector { // Event Bus Integration /// EventBusConnector for publishing events to the broker with WAL integration. event_bus_connector: Option, + + /// Shared executable-hash engine, cloned from the composition root. + /// + /// `None` means `--compute-hashes` is disabled; `Some` means the + /// post-enumeration hash pass runs on every collection cycle. The + /// engine is shared via `Arc` with `ProcessEventSource` so that a + /// single policy (one concurrency cap, one algorithm list) applies + /// regardless of which mode procmond is running in. See + /// `MultiAlgorithmHasher` rustdoc for the statelessness invariant + /// that protects cross-trust-domain sharing. + hasher: Option>, } impl ProcmondMonitorCollector { @@ -460,9 +472,34 @@ impl ProcmondMonitorCollector { pending_interval: None, // Event Bus Integration event_bus_connector: None, + hasher: None, }) } + /// Inject a shared executable-hash engine. + /// + /// The engine is cloned from the composition root in + /// `procmond/src/main.rs` and shared via `Arc` with any other holder + /// so that the concurrency cap, algorithm list, and size budget + /// apply as a single policy across the whole process. When `None` + /// (the default), the post-enumeration hash pass is skipped and + /// emitted events carry `executable_hash: None`. + #[must_use] + pub fn with_hasher(mut self, hasher: Option>) -> Self { + self.hasher = hasher; + self + } + + /// Returns a clone of the shared hasher `Arc`, if any. + /// + /// Intended for integration tests that need to assert `Arc::ptr_eq` + /// across multiple holders. Production code should not read the + /// engine through this accessor. + #[must_use] + pub fn hasher(&self) -> Option> { + self.hasher.as_ref().map(Arc::clone) + } + /// Creates a new actor channel and handle. /// /// This is a convenience method for creating the channel infrastructure. @@ -878,7 +915,7 @@ impl ProcmondMonitorCollector { ) .await; - let (process_events, _collection_stats) = match collection_result { + let (mut process_events, _collection_stats) = match collection_result { Ok(Ok((events, stats))) => (events, stats), Ok(Err(e)) => { error!(error = %e, "Process collection failed"); @@ -892,6 +929,54 @@ impl ProcmondMonitorCollector { } }; + // Populate `executable_hash` and `hash_algorithm` via the + // shared hash engine when `--compute-hashes` is enabled. The + // post-enumeration pass deduplicates unique paths so the cost + // is per-unique-executable, not per-process. Runs before + // lifecycle diffing so hashes participate in + // `(executable_hash, hash_algorithm)` tuple comparisons. + // + // Bounded by the REMAINING portion of the 30s cycle budget used + // by `collect_processes` above (see `CYCLE_BUDGET`). This keeps + // one actor cycle (enumeration + hash pass) from pinning the + // loop past the budget, which would delay GracefulShutdown, + // interval adjustments, and subsequent health traffic + // indefinitely on slow/hostile storage. + if let Some(ref hasher) = self.hasher { + const CYCLE_BUDGET: Duration = Duration::from_secs(30); + let remaining_budget = CYCLE_BUDGET.saturating_sub(collection_start.elapsed()); + if remaining_budget.is_zero() { + warn!( + cycle_budget_secs = CYCLE_BUDGET.as_secs(), + "skipping hash population: cycle budget exhausted by enumeration" + ); + } else { + match timeout( + remaining_budget, + populate_hashes(&mut process_events, hasher), + ) + .await + { + Ok(hash_stats) => { + debug!( + unique_paths = hash_stats.unique_paths, + hashed = hash_stats.hashed, + auth_failures = hash_stats.auth_failures, + io_failures = hash_stats.io_failures, + nonauthoritative = hash_stats.nonauthoritative, + "executable hash pass completed" + ); + } + Err(_elapsed) => { + warn!( + remaining_budget_ms = remaining_budget.as_millis(), + "hash population exceeded remaining cycle budget; partial coverage recorded" + ); + } + } + } + } + // Perform lifecycle analysis to detect process starts, stops, and modifications let lifecycle_events = { let mut tracker = self.lifecycle_tracker.lock().await; diff --git a/procmond/src/process_collector.rs b/procmond/src/process_collector.rs index 4bd64b49..7e469510 100644 --- a/procmond/src/process_collector.rs +++ b/procmond/src/process_collector.rs @@ -342,10 +342,12 @@ impl SysinfoProcessCollector { (None, None) }; - // Compute executable hash if requested - // TODO: Implement executable hashing (issue #40) - // For now, we'll leave this as None until the hashing implementation is added + // Executable hash is populated in a post-enumeration pass (see + // `hash_pass::populate_hashes`) so that the synchronous per-process + // conversion path stays off the async runtime. Leaving as None + // here; the post-pass rewrites events in place. let executable_hash: Option = None; + let hash_algorithm: Option = None; let user_id = process.user_id().map(|uid| uid.to_string()); let accessible = true; // Process is accessible if we can enumerate it @@ -361,6 +363,7 @@ impl SysinfoProcessCollector { cpu_usage, memory_usage, executable_hash, + hash_algorithm, user_id, accessible, file_exists, @@ -854,10 +857,10 @@ impl FallbackProcessCollector { (None, None, None) }; - // Compute executable hash if configured and path is available - // TODO: Implement executable hashing (issue #40) - // For now, we'll leave this as None until the hashing implementation is added + // Executable hash populated in a post-enumeration pass; see + // `hash_pass::populate_hashes`. let executable_hash: Option = None; + let hash_algorithm: Option = None; let user_id = process.user_id().map(|uid| uid.to_string()); let accessible = true; // Process is accessible if we can enumerate it @@ -873,6 +876,7 @@ impl FallbackProcessCollector { cpu_usage, memory_usage, executable_hash, + hash_algorithm, user_id, accessible, file_exists, diff --git a/procmond/src/wal.rs b/procmond/src/wal.rs index f32284d5..0d5979e4 100644 --- a/procmond/src/wal.rs +++ b/procmond/src/wal.rs @@ -1009,6 +1009,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, @@ -2718,6 +2719,7 @@ mod tests { cpu_usage: None, memory_usage: None, executable_hash: None, + hash_algorithm: None, user_id: None, accessible: true, file_exists: true, diff --git a/procmond/src/windows_collector.rs b/procmond/src/windows_collector.rs index b91b3ae5..94db1c17 100644 --- a/procmond/src/windows_collector.rs +++ b/procmond/src/windows_collector.rs @@ -1067,13 +1067,11 @@ impl WindowsProcessCollector { None }; - // Compute executable hash if requested - let executable_hash = if self.base_config.compute_executable_hashes { - // TODO: Implement executable hashing (issue #40) - None - } else { - None - }; + // Executable hash populated in a post-enumeration pass (see + // `hash_pass::populate_hashes`). Invariant: + // `executable_hash.is_some() == hash_algorithm.is_some()`. + let executable_hash: Option = None; + let hash_algorithm: Option = None; let user_id = process.user_id().map(|u| u.to_string()); let accessible = true; // If we can read process info, it's accessible @@ -1097,6 +1095,7 @@ impl WindowsProcessCollector { cpu_usage, memory_usage, executable_hash, + hash_algorithm, user_id, accessible, file_exists, diff --git a/procmond/tests/actor_mode_integration_tests.rs b/procmond/tests/actor_mode_integration_tests.rs index 5d8a2a5f..ec47ed72 100644 --- a/procmond/tests/actor_mode_integration_tests.rs +++ b/procmond/tests/actor_mode_integration_tests.rs @@ -80,6 +80,7 @@ fn create_test_process_event(pid: u32) -> ProcessEvent { cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123def456".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/procmond/tests/common/mod.rs b/procmond/tests/common/mod.rs index 110f318f..ded2f24b 100644 --- a/procmond/tests/common/mod.rs +++ b/procmond/tests/common/mod.rs @@ -37,7 +37,8 @@ pub fn create_test_event(pid: u32) -> ProcessEvent { start_time: Some(SystemTime::now()), cpu_usage: Some(5.0), memory_usage: Some(1024 * 1024), - executable_hash: Some(format!("hash_{pid}")), + executable_hash: Some(format!("{pid:064x}")), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -62,6 +63,7 @@ pub fn create_large_event(pid: u32, arg_count: usize) -> ProcessEvent { cpu_usage: Some(50.0), memory_usage: Some(100 * 1024 * 1024), executable_hash: Some("a".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("root".to_string()), accessible: true, file_exists: true, diff --git a/procmond/tests/event_bus_integration_tests.rs b/procmond/tests/event_bus_integration_tests.rs index 5d8dc046..448dc6d6 100644 --- a/procmond/tests/event_bus_integration_tests.rs +++ b/procmond/tests/event_bus_integration_tests.rs @@ -83,6 +83,7 @@ fn create_test_event(pid: u32) -> ProcessEvent { cpu_usage: Some(5.0), memory_usage: Some(1024 * 1024), executable_hash: Some(format!("hash_{pid}")), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -107,6 +108,7 @@ fn create_large_event(pid: u32, arg_count: usize) -> ProcessEvent { cpu_usage: Some(50.0), memory_usage: Some(100 * 1024 * 1024), executable_hash: Some("a".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("root".to_string()), accessible: true, file_exists: true, diff --git a/procmond/tests/hash_composition.rs b/procmond/tests/hash_composition.rs new file mode 100644 index 00000000..f19cc546 --- /dev/null +++ b/procmond/tests/hash_composition.rs @@ -0,0 +1,135 @@ +//! Composition-root integration test for the shared executable-hash engine. +//! +//! Verifies that when `procmond` constructs an `Arc` +//! and hands it to both the actor-mode `ProcmondMonitorCollector` and the +//! standalone `ProcessEventSource`, every holder ends up with the same +//! underlying allocation (`Arc::ptr_eq`). +//! +//! This is the explicit defense against the "silent no-op" regression +//! in the initial P1 binary-hashing implementation: before the +//! composition-root refactor, `--compute-hashes` was silently a no-op +//! because no single site constructed a `MultiAlgorithmHasher` and +//! threaded it into both the actor-mode collector and the standalone +//! event source. The test asserts the wiring cannot regress to that +//! state without a visible failure. +//! +//! See also: `docs/solutions/security-issues/binary-hashing-authorization-and-toctou-fixes.md` +//! for the full problem/solution writeup. + +#![allow( + clippy::expect_used, + clippy::unwrap_used, + clippy::panic, + clippy::uninlined_format_args +)] + +use daemoneye_lib::{ + integrity::{HasherConfig, MultiAlgorithmHasher}, + storage, +}; +use procmond::{ + ProcessEventSource, ProcessSourceConfig, + monitor_collector::{ProcmondMonitorCollector, ProcmondMonitorConfig}, + process_collector::ProcessCollectionConfig, +}; +use std::sync::Arc; +use tempfile::{TempDir, tempdir}; +use tokio::sync::{Mutex, mpsc}; + +/// Produce an in-memory-like `DatabaseManager` backed by a tempdir path. +/// +/// `storage::DatabaseManager::new` is the same entry point procmond uses +/// at startup; giving it a tempdir path exercises the real constructor +/// without touching `/var/lib/daemoneye`. +/// +/// Returns the [`TempDir`] alongside the database handle so the +/// underlying directory lives as long as the database — dropping the +/// `TempDir` before the handle would delete the backing files out from +/// under redb. +fn test_db() -> (TempDir, Arc>) { + let dir = tempdir().expect("tempdir"); + let path = dir.path().join("procmond-test.db"); + let db = Arc::new(Mutex::new( + storage::DatabaseManager::new(path.display().to_string()).expect("db new"), + )); + (dir, db) +} + +#[tokio::test] +async fn shared_arc_hasher_is_ptr_eq_across_holders() { + // Construct the composition root — exactly as `procmond/src/main.rs` + // does when `--compute-hashes == true`. + let engine = Arc::new( + MultiAlgorithmHasher::new(HasherConfig::default()).expect("engine constructs cleanly"), + ); + + // Inject into standalone-mode holder. Keep `_tmp_source` alive to + // preserve the backing tempdir for the database files. + let (_tmp_source, db_for_source) = test_db(); + let event_source = + ProcessEventSource::with_config(db_for_source, ProcessSourceConfig::default()) + .with_hasher(Some(Arc::clone(&engine))); + + // Inject into actor-mode holder. Keep `_tmp_actor` alive for the + // same reason. + let (_tmp_actor, db_for_actor) = test_db(); + let (_handle, rx) = ProcmondMonitorCollector::create_channel(); + let actor_config = ProcmondMonitorConfig { + process_config: ProcessCollectionConfig { + compute_executable_hashes: true, + ..Default::default() + }, + ..Default::default() + }; + let actor_collector = ProcmondMonitorCollector::new(db_for_actor, actor_config, rx) + .expect("actor collector constructs cleanly") + .with_hasher(Some(Arc::clone(&engine))); + + // Pull the engine clones back out of both holders and assert they + // point at the same allocation. This is the core invariant: a + // single policy (concurrency cap, algorithm list, size budget) + // applies to both procmond modes. + let engine_from_source = event_source.hasher().expect("standalone holder has engine"); + let engine_from_actor = actor_collector.hasher().expect("actor holder has engine"); + + assert!( + Arc::ptr_eq(&engine, &engine_from_source), + "standalone-mode holder received a different Arc" + ); + assert!( + Arc::ptr_eq(&engine, &engine_from_actor), + "actor-mode holder received a different Arc" + ); + assert!( + Arc::ptr_eq(&engine_from_source, &engine_from_actor), + "standalone and actor holders received different Arcs" + ); +} + +#[tokio::test] +async fn no_hasher_is_propagated_cleanly() { + // When --compute-hashes is OFF, both holders must see `None`. + // This guards against an accidental "default to Some" regression. + let (_tmp_source, db) = test_db(); + let source = ProcessEventSource::with_config(db, ProcessSourceConfig::default()); + assert!( + source.hasher().is_none(), + "ProcessEventSource::with_config must default to hasher: None" + ); + + let (_tmp_actor, db_for_actor) = test_db(); + let (_handle, rx) = ProcmondMonitorCollector::create_channel(); + let actor_collector = + ProcmondMonitorCollector::new(db_for_actor, ProcmondMonitorConfig::default(), rx) + .expect("actor constructs"); + assert!( + actor_collector.hasher().is_none(), + "ProcmondMonitorCollector::new must default to hasher: None" + ); +} + +// Suppress unused-import warning in environments where the mpsc +// re-export would otherwise be flagged. `mpsc` is used transitively via +// `ProcmondMonitorCollector::create_channel`. +#[allow(dead_code)] +fn _use_mpsc(_: mpsc::Sender) {} diff --git a/procmond/tests/lifecycle_integration_tests.rs b/procmond/tests/lifecycle_integration_tests.rs index e628a995..1363e38b 100644 --- a/procmond/tests/lifecycle_integration_tests.rs +++ b/procmond/tests/lifecycle_integration_tests.rs @@ -44,6 +44,7 @@ fn create_test_process_event( cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/procmond/tests/lifecycle_tracking_tests.rs b/procmond/tests/lifecycle_tracking_tests.rs index 434d32c2..97d21211 100644 --- a/procmond/tests/lifecycle_tracking_tests.rs +++ b/procmond/tests/lifecycle_tracking_tests.rs @@ -74,6 +74,7 @@ fn create_test_process_event( cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -98,6 +99,7 @@ fn create_process_event_with_start_time( cpu_usage: Some(1.0), memory_usage: Some(1024 * 1024), executable_hash: Some("abc123".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -230,6 +232,7 @@ fn test_start_detection_event_has_correct_metadata() { cpu_usage: Some(5.5), memory_usage: Some(50 * 1024 * 1024), // 50 MB executable_hash: Some("sha256:abc123def456".to_string()), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1001".to_string()), accessible: true, file_exists: true, @@ -1206,7 +1209,8 @@ fn test_snapshot_conversion_roundtrip() { start_time: Some(SystemTime::now() - Duration::from_secs(3600)), cpu_usage: Some(25.5), memory_usage: Some(256 * 1024 * 1024), - executable_hash: Some("sha256:fedcba987654321".to_string()), + executable_hash: Some("fedcba987654321".to_string()), + hash_algorithm: Some("sha256".to_string()), user_id: Some("user123".to_string()), accessible: true, file_exists: true, @@ -1230,6 +1234,7 @@ fn test_snapshot_conversion_roundtrip() { assert_eq!(original.cpu_usage, roundtrip.cpu_usage); assert_eq!(original.memory_usage, roundtrip.memory_usage); assert_eq!(original.executable_hash, roundtrip.executable_hash); + assert_eq!(original.hash_algorithm, roundtrip.hash_algorithm); assert_eq!(original.user_id, roundtrip.user_id); assert_eq!(original.accessible, roundtrip.accessible); assert_eq!(original.file_exists, roundtrip.file_exists); diff --git a/procmond/tests/load_tests.rs b/procmond/tests/load_tests.rs index 51e4e1b9..ddd7a557 100644 --- a/procmond/tests/load_tests.rs +++ b/procmond/tests/load_tests.rs @@ -50,6 +50,7 @@ fn create_test_event(pid: u32) -> ProcessEvent { cpu_usage: Some(1.0), memory_usage: Some(4096), executable_hash: None, + hash_algorithm: None, user_id: Some("1000".to_string()), accessible: true, file_exists: true, diff --git a/procmond/tests/security_tests.rs b/procmond/tests/security_tests.rs index 975d732c..bccf0f3a 100644 --- a/procmond/tests/security_tests.rs +++ b/procmond/tests/security_tests.rs @@ -94,6 +94,7 @@ fn create_test_event(pid: u32) -> ProcessEvent { cpu_usage: Some(5.0), memory_usage: Some(1024 * 1024), executable_hash: Some(format!("hash_{pid}")), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("1000".to_string()), accessible: true, file_exists: true, @@ -598,6 +599,7 @@ fn create_large_test_event(pid: u32, arg_count: usize) -> ProcessEvent { cpu_usage: Some(50.0), memory_usage: Some(100 * 1024 * 1024), executable_hash: Some("a".repeat(64)), + hash_algorithm: Some("sha256".to_owned()), user_id: Some("root".to_string()), accessible: true, file_exists: true, diff --git a/tessl.json b/tessl.json index a4cee386..dadeabf6 100644 --- a/tessl.json +++ b/tessl.json @@ -60,9 +60,6 @@ "cisco/software-security": { "version": "1.2.5" }, - "tessl-labs/good-oss-citizen": { - "version": "1.0.1" - }, "tessl-labs/skill-discovery": { "version": "0.27.0" }, @@ -71,6 +68,74 @@ }, "coding-agent-helpers/skeptic-verifier": { "version": "0.1.2" + }, + "neonwatty/logo-designer-skill": { + "version": "60285dd8417d5194155d8c2e349e2f8b61ffe6ff", + "source": "https://github.com/neonwatty/logo-designer-skill" + }, + "trailofbits/skills": { + "version": "d7f76b532d1e4c6e7757e04d25c99ab60dd5e32c", + "source": "https://github.com/trailofbits/skills", + "include": { + "skills": [ + "address-sanitizer", + "aflpp", + "ask-questions-if-underspecified", + "audit-augmentation", + "audit-context-building", + "audit-prep-assistant", + "cargo-fuzz", + "claude-in-chrome-troubleshooting", + "code-maturity-assessor", + "codeql", + "constant-time-analysis", + "constant-time-testing", + "coverage-analysis", + "crypto-protocol-diagram", + "debug-buttercup", + "designing-workflow-skills", + "devcontainer-setup", + "diagramming-code", + "differential-review", + "dimensional-analysis", + "dwarf-expert", + "entry-point-analyzer", + "fp-check", + "fuzzing-dictionary", + "fuzzing-obstacles", + "genotoxic", + "gh-cli", + "git-cleanup", + "graph-evolution", + "guidelines-advisor", + "harness-writing", + "insecure-defaults", + "libafl", + "mermaid-to-proverif", + "mutation-testing", + "ossfuzz", + "property-based-testing", + "sarif-parsing", + "seatbelt-sandboxer", + "second-opinion", + "secure-workflow-guide", + "semgrep", + "semgrep-rule-creator", + "semgrep-rule-variant-creator", + "sharp-edges", + "skill-improver", + "supply-chain-risk-auditor", + "testing-handbook-generator", + "trailmark", + "trailmark-structural", + "trailmark-summary", + "variant-analysis", + "vector-forge", + "wycheproof", + "yara-rule-authoring", + "zeroize-audit" + ] + } } } } diff --git a/yarn.lock b/yarn.lock deleted file mode 100644 index fb57ccd1..00000000 --- a/yarn.lock +++ /dev/null @@ -1,4 +0,0 @@ -# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. -# yarn lockfile v1 - -