Add: Qualcomm User Data Encryption test script & Document #141
Conversation
xbharani
commented
Aug 11, 2025
xbharani
commented
- Checks for fscryptctl binary presence
- Creates a random sw encryption key
- Applies and verifies encryption policy
- Confirms functionality with a test file
Runner/suites/Kernel/Baseport/UserDataEncryption/README_UserDataEncryption.md
Outdated
Show resolved
Hide resolved
|
This pull request has been marked as stale due to 30 days of inactivity. To prevent automatic closure in 7 days, remove the stale label or add a comment. You can reopen a closed pull request at any time. |
|
@xbharani Any update on the requested changes? |
|
This pull request has been marked as stale due to 30 days of inactivity. To prevent automatic closure in 7 days, remove the stale label or add a comment. You can reopen a closed pull request at any time. |
xbharani
force-pushed
the
main
branch
2 times, most recently
from
bd6eccf to
56d715d
Compare
xbharani
force-pushed
the
main
branch
2 times, most recently
from
686cf9f to
8f29437
Compare
- Checks for fscryptctl binary presence - Creates a random sw encryption key - Applies and verifies encryption policy - Confirms functionality with a test file - Added yaml config Signed-off-by: Bharani Bhuvanagiri <bbharani@qti.qualcomm.com>
smuppand
left a comment
smuppand
left a comment
There was a problem hiding this comment.
the script is close, but a few things will bite in CI / reliability, plus a couple of correctness/safety issues.
| exit 0 | ||
| fi | ||
|
|
||
| if [ -z "$__INIT_ENV_LOADED" ]; then |
There was a problem hiding this comment.
If the var is unset, some shells can still behave oddly, also it will trigger set -u issues if added later.
Use:
if [ -z "${__INIT_ENV_LOADED:-}" ]; then
| fi | ||
|
|
||
| log_info "Checking if dependency binary is available" | ||
| check_dependencies "$FSCRYPTCTL" |
There was a problem hiding this comment.
if check_dependencies returns non-zero you currently don’t handle it (you just call it).
Better to have an safer pattern.
|
|
||
| # Step 2: Create mount folder (this will create an unique folder under mnt) | ||
| log_info "Creating unique mount folder under /mnt" | ||
| MOUNT_DIR=$(mktemp -d /mnt/testing.XXXXXX) |
There was a problem hiding this comment.
mktemp under /mnt is not guaranteed to exist or be writable. On many images /mnt exists, but not always. Better:
Prefer ${TMPDIR:-/tmp} or ensure /mnt exists.
Example:
BASE_TMP="${TMPDIR:-/tmp}"
MOUNT_DIR="$(mktemp -d "$BASE_TMP/testing.XXXXXX")" || ...
|
|
||
| # Step 6: Verify policy | ||
| log_info "Verifying encryption policy" | ||
| policy_output=$("$FSCRYPTCTL" get_policy "$MOUNT_DIR" 2>/dev/null) |
There was a problem hiding this comment.
Your “encryption validation” is too weak (it will pass even if nothing is encrypted)
Right now you:
- set policy on an empty directory
- write a file
- read it back
That proves “filesystem writable”, not “data is encrypted”.
At minimum add one real validation:
Confirm the directory is marked encrypted via fscryptctl/lsattr output, OR
Check that get_policy returns non-empty + matches key id (you already do), and that encryption flags exist.
| fi | ||
|
|
||
| if [ -n "$key_id" ]; then | ||
| "$FSCRYPTCTL" remove_key "$key_id" "$FS_PATH" >/dev/null 2>&1 || true |
There was a problem hiding this comment.
Cleanup removes key even if FS_PATH is empty. If key_id is set but FS_PATH is empty (partial failure), you may call remove_key with bad args.
| log_warn "Failed to delete test file: $MOUNT_DIR/file.txt" | ||
| fi | ||
|
|
||
| if rmdir "$MOUNT_DIR" 2>/dev/null; then |
There was a problem hiding this comment.
rm / rmdir cleanup is fine, but you should not assume the file exists. Your logging is OK, but make it quieter.
Remove the “failed to delete” warnings when the file simply doesn’t exist.
Example:
[ -f "$MOUNT_DIR/file.txt" ] && rm -f "$MOUNT_DIR/file.txt" ...
| file_content=$(cat "$MOUNT_DIR/file.txt") | ||
| if [ "$file_content" = "file" ]; then | ||
| log_pass "$TESTNAME : Test Passed" | ||
| echo "$TESTNAME PASS" > "$res_file" |
| # Step 3: Add the key to the filesystem | ||
| log_info "Adding encryption key to the filesystem" | ||
| key_id=$("$FSCRYPTCTL" add_key "$FS_PATH" < "$KEY_FILE" 2>/dev/null) | ||
| scan_dmesg_errors "$SCRIPT_DIR" "fscrypt" "" |
There was a problem hiding this comment.
scan_dmesg_errors is aggressive, consider only scan at end, or scan with a narrow time window.
3 participants
