feat: change name to trustee_client, add systemd-cryptenroll and allow kbs_cert by file path#18
Open
litian1992 wants to merge 3 commits intolinux-system-roles:mainfrom
Open
feat: change name to trustee_client, add systemd-cryptenroll and allow kbs_cert by file path#18litian1992 wants to merge 3 commits intolinux-system-roles:mainfrom
litian1992 wants to merge 3 commits intolinux-system-roles:mainfrom
Conversation
The present name 'trustee_attestation_client' is too long. The keyword attestation is not obviously reflected in the role. Signed-off-by: Li Tian <litian@redhat.com>
Besides passing in the kbs cert file content, also allow passing in the file by its path. Signed-off-by: Li Tian <litian@redhat.com>
Reviewer's GuideRefactors the Ansible role from trustee_attestation_client to trustee_client across code, docs, CI, and tests, while adding TPM2-backed systemd-cryptenroll disk encryption support when the secret registration client is disabled and introducing support for providing the KBS certificate via either inline content or a file path, with corresponding tests and documentation updates. Sequence diagram for updated disk encryption flow with Secret Registration Client and systemd_cryptenrollsequenceDiagram
actor Admin
participant AnsibleController
participant trustee_client_role
participant SecretRegistrationClient
participant systemd_cryptenroll
participant TPM2
participant Cryptsetup
participant OS
Admin->>AnsibleController: Run playbook with trustee_client_encrypt_disk=true
AnsibleController->>trustee_client_role: Execute tasks/main.yml
trustee_client_role->>trustee_client_role: include_tasks encrypt_disk.yml
trustee_client_role->>trustee_client_role: Detect first unpartitioned disk
alt trustee_client_secret_registration_enabled=true
trustee_client_role->>SecretRegistrationClient: secret_registration_client.sh --fetch-key-to tempfile
SecretRegistrationClient-->>trustee_client_role: LUKS key written to tempfile
else trustee_client_secret_registration_enabled=false
trustee_client_role->>OS: type systemd-cryptenroll
OS-->>trustee_client_role: rc=0 if available
alt systemd-cryptenroll available
trustee_client_role->>OS: head -c 32 /dev/urandom | base64 > tempfile
OS-->>trustee_client_role: Random key stored in tempfile
end
end
trustee_client_role->>Cryptsetup: luksFormat --key-file tempfile disk_partition
Cryptsetup-->>trustee_client_role: LUKS container created
trustee_client_role->>Cryptsetup: open --key-file tempfile disk_partition encrypted_disk
Cryptsetup-->>trustee_client_role: /dev/mapper/encrypted_disk
trustee_client_role->>OS: mkfs.ext4 /dev/mapper/encrypted_disk
trustee_client_role->>OS: mkdir -p trustee_client_encrypt_disk_mount_point
trustee_client_role->>OS: mount /dev/mapper/encrypted_disk trustee_client_encrypt_disk_mount_point
alt trustee_client_secret_registration_enabled=false and systemd_cryptenroll available
trustee_client_role->>systemd_cryptenroll: Enroll TPM2 slot with unlock-key-file=tempfile
systemd_cryptenroll->>TPM2: Bind key to PCR7
TPM2-->>systemd_cryptenroll: TPM2 policy created
trustee_client_role->>systemd_cryptenroll: Wipe password slot
systemd_cryptenroll-->>trustee_client_role: LUKS updated
trustee_client_role->>OS: lsblk -dno UUID disk_partition
OS-->>trustee_client_role: UUID
trustee_client_role->>OS: Update /etc/crypttab with encrypted_disk UUID and tpm2-device=auto
trustee_client_role->>OS: Configure /etc/fstab and mount with x-systemd.requires=systemd-cryptsetup@encrypted_disk.service
end
trustee_client_role->>OS: Remove tempfile
OS-->>trustee_client_role: Key file deleted
trustee_client_role-->>AnsibleController: Disk encrypted and configured
AnsibleController-->>Admin: Playbook run complete
Flow diagram for updated task inclusion and disk encryption conditions in main roleflowchart TD
A_start["Start: tasks/main.yml"] --> B_quadlet["Include trustee_quadlet.yml\nwhen trustee_client_trustee_gc is true"]
B_quadlet --> C_secretReg["Include secret_registration_client.yml\nwhen trustee_client_secret_registration_enabled is true\nAND trustee_client_trustee_gc is true"]
C_secretReg --> D_encrypt["Include encrypt_disk.yml\nwhen trustee_client_encrypt_disk is true"]
B_quadlet --> D_encrypt
D_encrypt --> E_branch["Inside encrypt_disk.yml:\nIs there an unpartitioned disk?"]
E_branch -->|No| Z_end["End: no encryption performed"]
E_branch -->|Yes| F_checkSecretReg["Check trustee_client_secret_registration_enabled"]
F_checkSecretReg -->|true| G_useSecretReg["Use Secret Registration Client\nfetch key to tempfile"]
F_checkSecretReg -->|false| H_checkCryptenroll["Check for systemd-cryptenroll command"]
H_checkCryptenroll -->|Available| I_useTPM2["Generate random key to tempfile\nEncrypt disk and bind key via systemd-cryptenroll\nConfigure crypttab and fstab for auto-unlock"]
H_checkCryptenroll -->|Not available| J_encryptOnly["Encrypt and mount using key file\n(no TPM2 auto-unlock path)"]
G_useSecretReg --> K_encryptMount["Encrypt LUKS partition with fetched key\nand mount encrypted_disk"]
I_useTPM2 --> K_encryptMount
J_encryptOnly --> K_encryptMount
K_encryptMount --> L_cleanup["Remove key tempfile"]
L_cleanup --> Z_end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
encrypt_disk.yml, whentrustee_client_secret_registration_enabledis false andsystemd-cryptenrollis not available, the role still partitions, encrypts, and mounts the disk without configuring any auto-unlock mechanism; consider skipping disk encryption entirely in this case (or failing early with a clear message) to avoid leaving an unusable encrypted volume. - The
systemd-cryptenrollpresence check usescommand: type systemd-cryptenroll, which relies on shell built-ins and may be less portable; usingstaton a known path orcommand: systemd-cryptenroll --version(and checkingrc) would be more robust and explicit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `encrypt_disk.yml`, when `trustee_client_secret_registration_enabled` is false and `systemd-cryptenroll` is not available, the role still partitions, encrypts, and mounts the disk without configuring any auto-unlock mechanism; consider skipping disk encryption entirely in this case (or failing early with a clear message) to avoid leaving an unusable encrypted volume.
- The `systemd-cryptenroll` presence check uses `command: type systemd-cryptenroll`, which relies on shell built-ins and may be less portable; using `stat` on a known path or `command: systemd-cryptenroll --version` (and checking `rc`) would be more robust and explicit.
## Individual Comments
### Comment 1
<location path="tasks/encrypt_disk.yml" line_range="47-52" />
<code_context>
+ when: trustee_client_secret_registration_enabled | bool
+ no_log: true
+
+ - name: Check systemd-cryptenroll command exists
+ ansible.builtin.command: type systemd-cryptenroll
+ register: __trustee_client_cryptenroll_check
+ changed_when: false
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using `type systemd-cryptenroll` may be shell-dependent; consider a more portable existence check.
`ansible.builtin.command` runs via `/bin/sh`, and `type` isn’t portable across all shells or may not be available as a builtin, which can cause false negatives or different rc/output handling. Prefer a more portable check such as `command -v systemd-cryptenroll` or a direct path check with `stat` on `/usr/bin/systemd-cryptenroll` (or the expected location).
```suggestion
no_log: true
- name: Check systemd-cryptenroll command exists
ansible.builtin.command: command -v systemd-cryptenroll
register: __trustee_client_cryptenroll_check
changed_when: false
```
</issue_to_address>
### Comment 2
<location path="README.md" line_range="72" />
<code_context>
1. Finds the first unpartitioned and unmounted disk
-2. Requests disk encryption key from Secret Registration Client
-3. Encrypts the disk using above encryption key and mounts it at the designated path
+3. Encrypts the disk using key either from:
+ a. secret key fetched using Secret Registration Client (when enabled), or
+ b. `systemd-cryptenroll` which binds to PCR 7
</code_context>
<issue_to_address>
**nitpick (typo):** Minor grammar issue in "Encrypts the disk using key either from".
Consider rephrasing to: "Encrypts the disk using a key from either:" or "Encrypts the disk using the key from either:".
```suggestion
3. Encrypts the disk using a key from either:
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
litian1992
force-pushed
the
main
branch
2 times, most recently
from
e337166 to
6f8787a
Compare
richm
reviewed
Mar 19, 2026
richm
reviewed
Mar 19, 2026
tasks/encrypt_disk.yml
Outdated
| ansible.builtin.lineinfile: | ||
| path: /etc/crypttab | ||
| line: "encrypted_disk UUID={{ __luks_uuid.stdout }} none tpm2-device=auto" | ||
| regexp: '^encrypted_disk\s+' |
Contributor
There was a problem hiding this comment.
what if there is more than 1 encrypted_disk? or is that not possible?
Collaborator
Author
There was a problem hiding this comment.
how does trustee_client_encrypted_disk_0 sound?
richm
reviewed
Mar 19, 2026
richm
reviewed
Mar 19, 2026
| mode: "0600" | ||
|
|
||
| - name: Configure /etc/fstab and mount volume | ||
| ansible.builtin.mount: |
Contributor
There was a problem hiding this comment.
there is no builtin mount module - https://docs.ansible.com/projects/ansible/latest/collections/ansible/builtin/index.html#modules
Suggested change
| ansible.builtin.mount: | |
| ansible.posix.mount: |
Collaborator
Author
There was a problem hiding this comment.
Hmm, I'm a little surprised not catching error on this. But it did seem succeeding
# grep encrypt /etc/fstab
/dev/mapper/encrypted_disk /mnt/encrypted-disk ext4 defaults,x-systemd.requires=systemd-cryptsetup@encrypted_disk.service 0 0
Contributor
There was a problem hiding this comment.
A long time ago, the mount module was "built-in" (it was included with the Ansible 2.9 bundle before the ansible-core days), and Ansible still has some logic internally to redirect. However, it is fragile to rely on this implicit behavior - better to be explicit and use the real ansible.posix.mount module.
Contributor
|
@spetrosi are we going to rename the repo also? |
When Secret Registration Client is not enabled, allow disk encryption with systemd-cryptenroll. This binds disk encryption with TPM PCR7. Use crypttab and fstab for auto mount. Signed-off-by: Li Tian <litian@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enhancement:
trustee_client. Becausetrustee_attestation_clientis too long andattestationis not a key factor in this role.kbs_cert_content, also allow users to pass in the cert by path usingkbs_cert.systemd-cryptenrollcapability to encrypt_disk task when Secret Registration Client is not enabled.Reason:
Result:
Issue Tracker Tickets (Jira or BZ if any):
Summary by Sourcery
Rename the role and its variables from trustee_attestation_client to trustee_client, expand disk encryption support to use systemd-cryptenroll when the Secret Registration Client is disabled, and allow KBS certificates to be provided either as inline content or via a file path, updating tests and documentation accordingly.
New Features:
Enhancements:
Tests: