Skip to content

Add privilege dropping for push model agent#1162

Merged
sergio-correia merged 5 commits intokeylime:masterfrom
sergio-correia:run-as
Dec 10, 2025
Merged

Add privilege dropping for push model agent#1162
sergio-correia merged 5 commits intokeylime:masterfrom
sergio-correia:run-as

Conversation

@sergio-correia
Copy link
Copy Markdown
Contributor

@sergio-correia sergio-correia commented Dec 10, 2025

This PR implements privilege dropping support and related security enhancements for push model agent.

  • Add run_as configuration support for privilege dropping after initialization
  • Open privileged resources (IMA/UEFI logs) before dropping privileges
  • Use MeasurementList for IMA log reading instead of raw file access
  • Fix privilege dropping order to ensure secure initialization
  • Add KEYLIME_DIR environment variable support for verifier TLS certificates

Fixes #1157

The run_as() function had an incorrect order of operations when dropping
privileges, violating the security requirements specified in POS36-C and
potentially leaving the process with unintended supplementary group
privileges.

Previous (incorrect) order:
1. setgid() - Set GID
2. getgrouplist() - Get supplementary groups
3. setgroups() - Set supplementary groups
4. setuid() - Set UID

This order is unsafe because if setgroups() fails after setgid() succeeds,
the process retains supplementary group memberships from the root user,
potentially granting unintended privileges.

Corrected order (per POS36-C and CWE-696):
1. getgrouplist() - Get supplementary groups (while still root)
2. setgroups() - Drop supplementary groups FIRST
3. setgid() - Set GID SECOND
4. setuid() - Set UID LAST (irreversible)

This ensures that supplementary groups are cleared before changing the
primary GID, preventing privilege retention on error.

References:
- CERT C Secure Coding Standard: POS36-C
- CWE-696: Incorrect Behavior Order

Assisted-by: Claude 4 Sonnet
Signed-off-by: Sergio Correia <scorreia@redhat.com>
… model agent

This commit adds the infrastructure for proper privilege dropping in the
push model agent to match the security model of the pull model agent.

Changes:
- Add new privileged_resources module to handle root-required operations
- Open IMA and measured boot logs early in main() before dropping privileges
- Implement privilege dropping logic based on run_as config option
- Add comprehensive tests for privileged resource initialization

The privileged resources (IMA/measured boot log files) require root access
to /sys/kernel/security/. After opening these files, the agent drops
privileges to the configured user:group (default: keylime:keylime).
TPM access after privilege drop is provided via group membership (typically
the tss group).

The file handles are opened but not yet used by the attestation code.
Subsequent commits will thread these resources through the call chain.

Assisted-by: Claude 4 Sonnet
Signed-off-by: Sergio Correia <scorreia@redhat.com>
This commit threads PrivilegedResources through the attestation flow and
implements stateful IMA log reading using MeasurementList, ensuring IMA
evidence is available after privilege dropping.

- IMA evidence available after privilege dropping (uses pre-opened file handles)
- Stateful reading: only processes new entries since last read
- Avoids seeking issues on virtual filesystems (securityfs)
- More efficient than re-reading entire log each time
- Same proven approach as pull model agent

Assisted-by: Claude 4 Sonnet
Signed-off-by: Sergio Correia <scorreia@redhat.com>
@sergio-correia sergio-correia marked this pull request as draft December 10, 2025 10:35
…gent

The push model agent previously had hardcoded CLI argument defaults for
verifier TLS certificates that ignored the KEYLIME_DIR environment
variable. This change adds proper KEYLIME_DIR support by introducing new
config options that follow the same pattern used for registrar TLS
certificates.

Changes:
- Add verifier_tls_ca_cert, verifier_tls_client_cert, and
  verifier_tls_client_key config fields
- Make push model agent CLI args optional and use config values as
  defaults
- Update keylime-agent.conf with documentation for new options
- Path resolution now respects KEYLIME_DIR for verifier certificates

Backward compatibility:
- CLI arguments continue to work and override config values
- Without KEYLIME_DIR, paths resolve to /var/lib/keylime/cv_ca/* (same
  as before)
- With KEYLIME_DIR, paths resolve to $KEYLIME_DIR/cv_ca/* (new
  capability)

Assisted-by: Claude 4.5 Sonnet
Signed-off-by: Sergio Correia <scorreia@redhat.com>
Signed-off-by: Sergio Correia <scorreia@redhat.com>
@sergio-correia sergio-correia marked this pull request as ready for review December 10, 2025 12:15
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 10, 2025

Codecov Report

❌ Patch coverage is 25.23077% with 243 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.33%. Comparing base (4ef7acb) to head (2c430d3).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
keylime-push-model-agent/src/main.rs 6.06% 62 Missing ⚠️
keylime-push-model-agent/src/struct_filler.rs 13.11% 53 Missing ⚠️
keylime/src/context_info.rs 12.90% 27 Missing ⚠️
...ylime-push-model-agent/src/privileged_resources.rs 45.65% 25 Missing ⚠️
keylime/src/permissions.rs 26.66% 22 Missing ⚠️
keylime/src/uefi/uefi_log_handler.rs 0.00% 20 Missing ⚠️
...ylime-push-model-agent/src/context_info_handler.rs 10.00% 9 Missing ⚠️
keylime-push-model-agent/src/attestation.rs 20.00% 8 Missing ⚠️
keylime-push-model-agent/src/state_machine.rs 0.00% 7 Missing ⚠️
keylime-push-model-agent/src/url_selector.rs 60.00% 4 Missing ⚠️
... and 3 more
Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 57.33% <25.23%> (-0.91%) ⬇️
upstream-unit-tests 57.33% <25.23%> (-0.91%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
keylime/src/config/base.rs 88.02% <100.00%> (+0.66%) ⬆️
keylime/src/config/env.rs 84.21% <ø> (ø)
keylime/src/config/push_model.rs 60.00% <ø> (ø)
keylime/src/error.rs 9.02% <100.00%> (ø)
keylime/src/registrar_client.rs 82.87% <100.00%> (ø)
keylime-push-model-agent/src/header_validation.rs 53.48% <33.33%> (ø)
keylime/src/auth.rs 44.86% <0.00%> (+0.27%) ⬆️
keylime/src/resilient_client.rs 70.07% <66.66%> (-0.36%) ⬇️
keylime-push-model-agent/src/url_selector.rs 71.42% <60.00%> (+4.39%) ⬆️
keylime-push-model-agent/src/state_machine.rs 17.36% <0.00%> (-0.57%) ⬇️
... and 8 more

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sergio-correia sergio-correia merged commit bc586bd into keylime:master Dec 10, 2025
18 of 19 checks passed
@ansasaki ansasaki mentioned this pull request Jan 28, 2026
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

push agent not following run_as settings

2 participants