Skip to content

fix(push-model): resolve hash_ek uuid to actual EK hash#1176

Merged
ansasaki merged 1 commit intokeylime:masterfrom
Nordix:tuomo/fix-hash-ek-on-push-model
Jan 13, 2026
Merged

fix(push-model): resolve hash_ek uuid to actual EK hash#1176
ansasaki merged 1 commit intokeylime:masterfrom
Nordix:tuomo/fix-hash-ek-on-push-model

Conversation

@tuminoid
Copy link
Copy Markdown
Contributor

@tuminoid tuminoid commented Jan 9, 2026

When uuid config is set to 'hash_ek', the push model agent now correctly uses the computed ek_hash from the TPM context info instead of the literal string 'hash_ek'.

This aligns the push model agent behavior with the regular agent which already handles this case in keylime-agent/src/main.rs.

@tuminoid tuminoid force-pushed the tuomo/fix-hash-ek-on-push-model branch 2 times, most recently from e5faf2f to 75e65af Compare January 9, 2026 08:56
@tuminoid
Copy link
Copy Markdown
Contributor Author

tuminoid commented Jan 9, 2026

I'm testing the push-model agent setup in k8s and we're using hash_ek UUIDs. Found this implementation gap between push-model agent and pull-model agent. PTAL @ansasaki @sarroutbi @sergio-correia

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.

  1. Centralize the UUID Resolution (DRY Principle)

You currently have the "if hash_ek then ek_hash" logic duplicated in both main.rs and registration.rs.

Risk: If the logic for resolving identifiers changes (e.g., adding support for a new dynamic ID type), you have to update it in two places.

Suggestion: Add a helper method to the ContextInfo struct (in keylime/src/context_info.rs) or a local helper in the agent to resolve the identifier.

Example Helper:

fn resolve_agent_id(config_uuid: &str, ctx_info: &ContextInfo) -> String {
    if config_uuid == "hash_ek" {
        ctx_info.ek_hash.clone()
    } else {
        config_uuid.to_string()
    }
}
  1. Case Sensitivity and Whitespace
    The current check config.uuid() == "hash_ek" is very strict. Users occasionally add trailing spaces in config files or use capital letters.

Suggestion: Use .trim().to_lowercase() to make the configuration more resilient.

let uuid_config = config.uuid().trim().to_lowercase();
if uuid_config == "hash_ek" { ... }

@tuminoid tuminoid force-pushed the tuomo/fix-hash-ek-on-push-model branch 2 times, most recently from 0f629d5 to 9f3a9b1 Compare January 12, 2026 09:34
@tuminoid
Copy link
Copy Markdown
Contributor Author

Thanks for the review @sarroutbi. Updated and rebased.

@ansasaki
Copy link
Copy Markdown
Contributor

/packit test

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
Copy link
Copy Markdown
Contributor

LGTM. It would probably be nice to have some test for the new resolve_agent_id() helper, but we can do that later. On another note, what is happening to the e2e tests? Would you know if we have some outage, @kkaarreell?

@kkaarreell
Copy link
Copy Markdown
Contributor

Would you know if we have some outage, @kkaarreell?

Packit had an outage during the night and now it is processing the queue.

@tuminoid
Copy link
Copy Markdown
Contributor Author

LGTM. It would probably be nice to have some test for the new resolve_agent_id() helper, but we can do that later. On another note, what is happening to the e2e tests? Would you know if we have some outage, @kkaarreell?

I will add tests in follow-up, no problem.

Comment thread keylime-push-model-agent/src/main.rs
Comment thread keylime/src/context_info.rs Outdated
@ansasaki
Copy link
Copy Markdown
Contributor

The error on Fedora 42 looks unrelated (core dump on mariadb):

Redirecting to /bin/systemctl start mariadb.service
Job for mariadb.service failed because a fatal signal was delivered causing the control process to dump core.
See "systemctl status mariadb.service" and "journalctl -xeu mariadb.service" for details.
:: [ 13:24:48 ] :: [   ERROR  ] :: rlServiceStart: Starting service mariadb failed
:: [ 13:24:48 ] :: [   ERROR  ] :: Status of the failed service:
:: [ 13:24:48 ] :: [   LOG    ] ::   Redirecting to /bin/systemctl status mariadb.service
:: [ 13:24:48 ] :: [   LOG    ] ::   â—� mariadb.service - MariaDB 10.11 database server
:: [ 13:24:48 ] :: [   LOG    ] ::   Loaded: loaded (/usr/lib/systemd/system/mariadb.service; disabled; preset: disabled)
:: [ 13:24:48 ] :: [   LOG    ] ::   Drop-In: /usr/lib/systemd/system/service.d
:: [ 13:24:48 ] :: [   LOG    ] ::   └─10-timeout-abort.conf
:: [ 13:24:48 ] :: [   LOG    ] ::   Active: activating (auto-restart) (Result: core-dump) since Mon 2026-01-12 13:24:48 UTC; 41ms ago
:: [ 13:24:48 ] :: [   LOG    ] ::   Invocation: 6f55fe450b0b495daee99f342b48c800
:: [ 13:24:48 ] :: [   LOG    ] ::   Docs: man:mariadbd(8)
:: [ 13:24:48 ] :: [   LOG    ] ::   https://mariadb.com/kb/en/library/systemd/
:: [ 13:24:48 ] :: [   LOG    ] ::   Process: 100158 ExecStartPre=/usr/libexec/mariadb-check-socket (code=exited, status=0/SUCCESS)
:: [ 13:24:48 ] :: [   LOG    ] ::   Process: 100181 ExecStartPre=/usr/libexec/mariadb-prepare-db-dir mariadb.service (code=exited, status=0/SUCCESS)
:: [ 13:24:48 ] :: [   LOG    ] ::   Process: 100266 ExecStart=/usr/libexec/mariadbd --basedir=/usr $MYSQLD_OPTS $_WSREP_NEW_CLUSTER (code=dumped, signal=ABRT)
:: [ 13:24:48 ] :: [   LOG    ] ::   Main PID: 100266 (code=dumped, signal=ABRT)
:: [ 13:24:48 ] :: [   LOG    ] ::   Mem peak: 140.4M
:: [ 13:24:48 ] :: [   LOG    ] ::   CPU: 1.393s

When uuid config is set to 'hash_ek', the push model agent now
correctly uses the computed ek_hash from the TPM context info
instead of the literal string 'hash_ek'.

This aligns the push model agent behavior with the regular agent
which already handles this case in keylime-agent/src/main.rs.

Signed-off-by: Tuomo Tanskanen <tuomo.tanskanen@est.tech>
@tuminoid tuminoid force-pushed the tuomo/fix-hash-ek-on-push-model branch from 9f3a9b1 to 34c4337 Compare January 13, 2026 07:16
@tuminoid
Copy link
Copy Markdown
Contributor Author

Fixed @ansasaki's trim() comments, and rebased.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 13, 2026

Codecov Report

❌ Patch coverage is 39.13043% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.16%. Comparing base (6dcf91e) to head (34c4337).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
keylime-push-model-agent/src/main.rs 21.42% 11 Missing ⚠️
keylime/src/context_info.rs 66.66% 2 Missing ⚠️
keylime-push-model-agent/src/registration.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
Flag Coverage Δ
e2e-testsuite 37.38% <0.00%> (-0.07%) ⬇️
upstream-unit-tests 65.98% <52.94%> (-0.05%) ⬇️

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

Files with missing lines Coverage Δ
keylime-push-model-agent/src/registration.rs 54.34% <66.66%> (-0.60%) ⬇️
keylime/src/context_info.rs 45.39% <66.66%> (+0.27%) ⬆️
keylime-push-model-agent/src/main.rs 27.09% <21.42%> (-0.81%) ⬇️

... and 5 files with indirect coverage changes

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

@ansasaki
Copy link
Copy Markdown
Contributor

@tuminoid Thank you for your contribution!

@ansasaki ansasaki merged commit 8730c4c into keylime:master Jan 13, 2026
13 of 15 checks passed
@tuminoid
Copy link
Copy Markdown
Contributor Author

@tuminoid Thank you for your contribution!

Thanks for quick reviews @ansasaki, @sergio-correia and @sarroutbi !

I will follow-up with unit tests for the new util function.

@tuminoid tuminoid deleted the tuomo/fix-hash-ek-on-push-model branch January 13, 2026 12:19
@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.

5 participants