feat(secret_registration_client): add secret registration client service#16
Merged
richm merged 2 commits intolinux-system-roles:mainfrom Mar 18, 2026
Merged
Conversation
Do not run trustee guest pod on encrypted disk overlay. So that there's no SELinux violation. Signed-off-by: Li Tian <litian@redhat.com>
Reviewer's GuideIntroduces a secret registration client service that integrates with Trustee GC to obtain and use a remote disk-encryption key, rewires the encrypt-disk workflow to depend on it, writes the KBS certificate to disk, removes SELinux disabling and Podman storage-on-encrypted-disk logic, and extends tests/docs/defaults accordingly. Sequence diagram for secret registration and key fetch during disk encryptionsequenceDiagram
participant AnsibleRole
participant EncryptDiskTask
participant SecretRegistrationClientScript as secret_registration_client.sh
participant CDH as ConfidentialDataHub_CDH
participant RegServer as SecretRegistrationServer
participant KBS as Trustee_KBS
AnsibleRole->>EncryptDiskTask: include_tasks encrypt_disk.yml
EncryptDiskTask->>EncryptDiskTask: Detect unpartitioned_disk
EncryptDiskTask->>SecretRegistrationClientScript: --fetch-key-to /tmp/secret_key.bin
SecretRegistrationClientScript->>CDH: GET /cdh/resource/disk-encryption/{client_id}/luks-key-0
alt Key already registered
CDH-->>SecretRegistrationClientScript: 200 OK, key
else Key not found
SecretRegistrationClientScript->>CDH: GET /aa/token?token_type=kbs
CDH-->>SecretRegistrationClientScript: 200 OK, attestation_token
SecretRegistrationClientScript->>RegServer: POST /register-encryption-key
Note right of RegServer: Body includes attestation_token and client_id
RegServer->>KBS: Store key bound to client_id
KBS-->>RegServer: 200 OK
RegServer-->>SecretRegistrationClientScript: 200 OK
SecretRegistrationClientScript->>CDH: GET /cdh/resource/disk-encryption/{client_id}/luks-key-0
CDH-->>SecretRegistrationClientScript: 200 OK, key
end
SecretRegistrationClientScript-->>EncryptDiskTask: Write key to /tmp/secret_key.bin
EncryptDiskTask->>EncryptDiskTask: parted mklabel/mkpart
EncryptDiskTask->>EncryptDiskTask: cryptsetup luksFormat --key-file /tmp/secret_key.bin
EncryptDiskTask->>EncryptDiskTask: cryptsetup open encrypted-disk
EncryptDiskTask->>EncryptDiskTask: mkfs.ext4 /dev/mapper/encrypted-disk
EncryptDiskTask->>EncryptDiskTask: mount /dev/mapper/encrypted-disk to mount_point
EncryptDiskTask->>EncryptDiskTask: Remove /tmp/secret_key.bin
Sequence diagram for secret registration client systemd service at bootsequenceDiagram
participant Systemd as systemd
participant SecretRegService as secret_registration_client.service
participant SecretRegistrationClientScript as secret_registration_client.sh
participant CDH as ConfidentialDataHub_CDH
participant RegServer as SecretRegistrationServer
participant KBS as Trustee_KBS
participant LUKSPartition as LUKS_partition
Systemd->>SecretRegService: Start (After network-online and trustee-gc-pod)
SecretRegService->>SecretRegistrationClientScript: ExecStart --mount-disk
SecretRegistrationClientScript->>SecretRegistrationClientScript: Check mountpoint mounted
alt Already mounted
SecretRegistrationClientScript-->>SecretRegService: Exit success
SecretRegService-->>Systemd: Active (exited)
else Not mounted
SecretRegistrationClientScript->>LUKSPartition: Scan blkid for TYPE=crypto_LUKS
LUKSPartition-->>SecretRegistrationClientScript: First unmounted LUKS device
SecretRegistrationClientScript->>SecretRegistrationClientScript: mktemp key_path
SecretRegistrationClientScript->>CDH: GET /cdh/resource/disk-encryption/{client_id}/luks-key-0
alt Key already present
CDH-->>SecretRegistrationClientScript: 200 OK, key
else Key not present
SecretRegistrationClientScript->>CDH: GET /aa/token?token_type=kbs
CDH-->>SecretRegistrationClientScript: 200 OK, attestation_token
SecretRegistrationClientScript->>RegServer: POST /register-encryption-key
Note right of RegServer: Body includes attestation_token and client_id
RegServer->>KBS: Store or provision key
KBS-->>RegServer: 200 OK
RegServer-->>SecretRegistrationClientScript: 200 OK
SecretRegistrationClientScript->>CDH: GET /cdh/resource/disk-encryption/{client_id}/luks-key-0
CDH-->>SecretRegistrationClientScript: 200 OK, key
end
SecretRegistrationClientScript->>LUKSPartition: cryptsetup open --key-file key_path encrypted-disk
SecretRegistrationClientScript->>LUKSPartition: mount /dev/mapper/encrypted-disk to MOUNT_POINT
SecretRegistrationClientScript->>SecretRegistrationClientScript: Remove key_path
SecretRegistrationClientScript-->>SecretRegService: Exit success
SecretRegService-->>Systemd: Active (exited, RemainAfterExit=yes)
end
Flow diagram for updated Ansible role task orchestrationflowchart TD
A[Set_platform_and_version_specific_variables
tasks_set_vars.yml] --> B{trustee_attestation_client_trustee_gc}
B -- true --> C[Deploy_Trustee_Guest_Components
trustee_quadlet.yml]
B -- false --> D[Skip_Trustee_GC_deployment]
C --> E{trustee_attestation_client_secret_registration_client_enabled}
D --> E
E -- true --> F[Deploy_Secret_Registration_Client_Service
secret_registration_client.yml]
E -- false --> G[Skip_Secret_Registration_Client]
F --> H{trustee_attestation_client_encrypt_disk
and
trustee_attestation_client_secret_registration_client_enabled}
G --> H
H -- true --> I[Create_encrypted_disk_partition
encrypt_disk.yml]
H -- false --> J[Skip_disk_encryption]
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 7 issues, and left some high level feedback:
- In
secret_registration_client.sh, the wayREGISTRATION_SERVER_URLandREGISTRATION_SERVER_PORTare combined (${REGISTRATION_SERVER_URL%:*}:${REGISTRATION_SERVER_PORT}) assumes a simple host:port URL and will behave unexpectedly if the KBS URL includes a path, scheme-only URL, or no port; consider parsing or configuring the registration endpoint separately instead of string-mangling the URL. - The encrypt-disk flow uses a fixed path (
/tmp/secret_key.bin) for the fetched key, while the client script usesmktempfor the same purpose; for consistency and to reduce the risk of races or permission issues, consider switchingencrypt_disk.ymlto use a securely created temp file as well.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `secret_registration_client.sh`, the way `REGISTRATION_SERVER_URL` and `REGISTRATION_SERVER_PORT` are combined (`${REGISTRATION_SERVER_URL%:*}:${REGISTRATION_SERVER_PORT}`) assumes a simple host:port URL and will behave unexpectedly if the KBS URL includes a path, scheme-only URL, or no port; consider parsing or configuring the registration endpoint separately instead of string-mangling the URL.
- The encrypt-disk flow uses a fixed path (`/tmp/secret_key.bin`) for the fetched key, while the client script uses `mktemp` for the same purpose; for consistency and to reduce the risk of races or permission issues, consider switching `encrypt_disk.yml` to use a securely created temp file as well.
## Individual Comments
### Comment 1
<location path="tasks/trustee_quadlet.yml" line_range="123-127" />
<code_context>
when: trustee_gc_pod_name.files | length > 0
failed_when: false
+- name: Write KBS certificate to /etc/trustee-gc/server.crt
+ ansible.builtin.copy:
+ content: "{{ trustee_attestation_client_trustee_kbs_cert }}"
+ dest: /etc/trustee-gc/server.crt
+ mode: "0644"
+ when: trustee_attestation_client_trustee_kbs_cert | length > 0
+
</code_context>
<issue_to_address>
**issue (bug_risk):** KBS certificate variable is treated as inline content but example still uses it as a file path, likely breaking TLS to KBS.
This task now assumes `trustee_attestation_client_trustee_kbs_cert` contains PEM content (`content: ...`), but `examples/simple.yml` still sets it to a file path. That will write the literal path into `/etc/trustee-gc/server.crt`, causing `curl --cacert /etc/trustee-gc/server.crt` in `secret_registration_client.sh` to fail TLS verification. Please either (a) switch to `src:` and keep the variable as a path, or (b) update defaults/examples/docs to require PEM content and rename the variable to reflect that.
</issue_to_address>
### Comment 2
<location path="tasks/main.yml" line_range="10-12" />
<code_context>
include_tasks: trustee_quadlet.yml
when: trustee_attestation_client_trustee_gc | bool
+
+- name: Deploy Secret Registration Client Service
+ include_tasks: secret_registration_client.yml
+ when: trustee_attestation_client_secret_registration_client_enabled | bool
+
+- name: Create encrypted disk partition
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Secret registration client is deployed without enforcing the documented dependency on Trustee GC.
`secret_registration_client.yml` documents a dependency on Trustee GC (`trustee_attestation_client_trustee_gc`), but this task is only gated on `trustee_attestation_client_secret_registration_client_enabled`. This allows enabling the client without GC/CDH deployed, leading to runtime failure when calling `${CDH_URL}`. Please also require `trustee_attestation_client_trustee_gc | bool` (or add a fail-fast guard) so misconfiguration is caught early.
</issue_to_address>
### Comment 3
<location path="tasks/encrypt_disk.yml" line_range="34-35" />
<code_context>
- name: Set fact with disk device path and partition path
ansible.builtin.set_fact:
disk_device: "/dev/{{ unpartitioned_disk.stdout }}"
+ secret_key_file: "/tmp/secret_key.bin"
disk_partition: >-
/dev/{{ unpartitioned_disk.stdout }}{{ 'p' if 'nvme' in unpartitioned_disk.stdout else '' }}1
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Using a fixed /tmp/secret_key.bin path for the disk key introduces an avoidable security and concurrency risk.
This hard-coded `secret_key_file` path leaves a predictable filename in `/tmp`, creates a race window before cleanup, and prevents safe concurrent runs. Prefer `ansible.builtin.tempfile` (or another per-run unique path) and pass that into `secret_registration_client.sh --fetch-key-to`, then remove it after use.
Suggested implementation:
```
- name: Create temporary file for disk encryption key
ansible.builtin.tempfile:
state: file
suffix: secret_key.bin
register: secret_key_tmp
- name: Set fact with disk device path and partition path
ansible.builtin.set_fact:
disk_device: "/dev/{{ unpartitioned_disk.stdout }}"
secret_key_file: "{{ secret_key_tmp.path }}"
disk_partition: >-
/dev/{{ unpartitioned_disk.stdout }}{{ 'p' if 'nvme' in unpartitioned_disk.stdout else '' }}1
- name: Acquire disk encryption key from Secret Registration Client service
ansible.builtin.shell: |
/usr/local/bin/secret_registration_client.sh --fetch-key-to {{ secret_key_file }}
changed_when: true
- name: Remove temporary disk encryption key file
ansible.builtin.file:
path: "{{ secret_key_file }}"
state: absent
```
If the `secret_key_file` is used by additional tasks later in `tasks/encrypt_disk.yml` (for example, passed to a disk encryption command), move the "Remove temporary disk encryption key file" task to immediately after the final use of `secret_key_file` so the key remains available for all required operations but is still cleaned up promptly afterward.
</issue_to_address>
### Comment 4
<location path="README.md" line_range="56" />
<code_context>
+
+1. Downloads the Podman Quadlets from designated repo
+2. Configures the settings in /etc/trustee-gc/
+3. Enables and start trustee-gc.pod as a service
+
+## Secret Registration Client
</code_context>
<issue_to_address>
**issue (typo):** Fix verb agreement in 'Enables and start'.
Change to `Enables and starts trustee-gc.pod as a service` so the verbs agree grammatically.
```suggestion
3. Enables and starts trustee-gc.pod as a service
```
</issue_to_address>
### Comment 5
<location path="README.md" line_range="62" />
<code_context>
+
+When enabled, this task:
+
+1. Sends registation request to Secret Registration Server via HTTPS to acquire disk encryption keys
+2. Requests above disk encryption key upon boot when Encrypt Disk is enabled to decrypt and mount disk
+
</code_context>
<issue_to_address>
**issue (typo):** Correct the spelling of 'registation' to 'registration' and consider adding an article.
For example: `Sends a registration request to the Secret Registration Server via HTTPS to acquire disk encryption keys.`
</issue_to_address>
### Comment 6
<location path="README.md" line_range="63" />
<code_context>
+When enabled, this task:
+
+1. Sends registation request to Secret Registration Server via HTTPS to acquire disk encryption keys
+2. Requests above disk encryption key upon boot when Encrypt Disk is enabled to decrypt and mount disk
+
+## Encrypt Disk
</code_context>
<issue_to_address>
**suggestion (typo):** Improve wording by adding missing articles for clarity.
Consider rephrasing this line to: `Requests the above disk encryption key upon boot when Encrypt Disk is enabled, to decrypt and mount the disk.`
```suggestion
2. Requests the above disk encryption key upon boot when Encrypt Disk is enabled, to decrypt and mount the disk
```
</issue_to_address>
### Comment 7
<location path="README.md" line_range="71" />
<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 mount to designated path
+
## License
</code_context>
<issue_to_address>
**issue (typo):** Fix grammar in the phrase about encrypting and mounting the disk.
Change to `Encrypts the disk using the above encryption key and mounts it at the designated path` to fix verb agreement and the missing article.
```suggestion
3. Encrypts the disk using the above encryption key and mounts it at the designated path
```
</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
trustee_client
branch
2 times, most recently
from
5f87fcd to
4e4d99b
Compare
spetrosi
reviewed
Mar 11, 2026
richm
reviewed
Mar 11, 2026
richm
reviewed
Mar 11, 2026
richm
reviewed
Mar 11, 2026
litian1992
force-pushed
the
trustee_client
branch
3 times, most recently
from
723c008 to
8bfce2e
Compare
richm
reviewed
Mar 16, 2026
litian1992
force-pushed
the
trustee_client
branch
from
8bfce2e to
abac0c0
Compare
richm
reviewed
Mar 17, 2026
richm
reviewed
Mar 17, 2026
richm
reviewed
Mar 17, 2026
Contributor
|
there are a couple of files that do not end with a newline |
…ice creation The secret registration client service is the counterpart of the server service. It does 1. register the trustee client with server by an attestation. 2. fetch the key created by server service and mount the encrypted disk. Signed-off-by: Li Tian <litian@redhat.com>
litian1992
force-pushed
the
trustee_client
branch
from
abac0c0 to
897faad
Compare
Contributor
|
lgtm - @spetrosi ? |
spetrosi
approved these changes
Mar 18, 2026
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
3 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:
Secret registration client serves as a companion to Trustee client that registers the CVM to Trustee server. This way Trustee server can serve each CVM as individuals. This secret registration client is also responsible for fetching key and mount the encrypted data disk at boot.
Additionally, drop SELinux disablement since the issue is fixed in quadlet.
Reason:
Result:
Issue Tracker Tickets (Jira or BZ if any):
Summary by Sourcery
Introduce a secret registration client service that integrates with Trustee guest components to manage disk encryption keys and automate encrypted disk mounting, while simplifying KBS certificate handling and removing obsolete SELinux handling.
New Features:
Enhancements:
Tests: