Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(RHEL-50672) Harden /bin/kernel-install to avoid failing silently when /etc/machine-id doesn't end with newline #290

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

dtardon
Copy link
Member

@dtardon dtardon commented Jul 31, 2024

Resolves: RHEL-50672

@github-actions github-actions bot changed the title Harden /bin/kernel-install to avoid failing silently when /etc/machine-id doesn't end with newline (RHEL-50672) Harden /bin/kernel-install to avoid failing silently when /etc/machine-id doesn't end with newline Jul 31, 2024
@github-actions github-actions bot added pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Jul 31, 2024
Copy link

github-actions bot commented Jul 31, 2024

Commit validation

Tracker - RHEL-50672

The following commits meet all requirements

commit upstream
5e2e144 - kernel-install: check machine ID RHEL-only: feature
48eac39 - kernel-install: ignore errors when reading /etc/machine-id RHEL-only: feature

Tracker validation

Success

🟢 Tracker RHEL-50672 has set desired product: rhel-9.4
🟢 Tracker RHEL-50672 has set desired component: systemd
🟢 Tracker RHEL-50672 has been approved


Pull Request validation

Success

🟢 CI - All checks have passed
🟢 Review - Reviewed by a member
🟢 Approval - Changes were approved


Auto Merge

Success

🟢 Pull Request is not marked as draft and it's not blocked by dont-merge label
🟢 Pull Request meet requirements, title has correct form
🟢 Pull Request meet requirements, mergeable is true
🟢 Pull Request meet requirements, mergeable_state is clean
🟢 Pull Request has correct target branch main
🟢 Pull Request was merged

@jamacku jamacku added this to the RHEL-9.5.0 milestone Jul 31, 2024
dtardon added 2 commits August 1, 2024 12:29
Let's do at least a minimal check that the read machine ID is valid.

RHEL-only: feature

Related: RHEL-50672
If /etc/machine-id doesn't end with a newline (which is wrong, but it
can happen), kernel-install fails. That's because `read` returns 1 if it
reaches EOF and we're using `set -e`. Let's just ignore that error, as
it doesn't matter: we have either read a valid machine ID, in which case
we continue, or we haven't, in which case we exit anyway.

RHEL-only: feature

Resolves: RHEL-50672
@dtardon dtardon force-pushed the RHEL-50672-machine-id branch from c53d2b6 to 48eac39 Compare August 1, 2024 10:29
@github-actions github-actions bot removed the pr/needs-ci Formerly needs-ci label Aug 1, 2024
@jamacku jamacku requested a review from msekletar August 1, 2024 13:12
Copy link
Member

@msekletar msekletar left a comment

Choose a reason for hiding this comment

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

LGTM

[ "$MACHINE_ID" = "uninitialized" ] && unset MACHINE_ID
if [ "$MACHINE_ID" = "uninitialized" ]; then
unset MACHINE_ID
elif [ {{ '${#MACHINE_ID}' }} -ne 32 ]; then
Copy link
Member

Choose a reason for hiding this comment

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

Took me a bit to realize why this is needed, those pesky Jinja comments :)

@github-actions github-actions bot removed the pr/needs-review Formerly needs-review label Aug 1, 2024
@github-actions github-actions bot merged commit 86572fe into redhat-plumbers:main Aug 1, 2024
30 checks passed
@dtardon dtardon deleted the RHEL-50672-machine-id branch August 2, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants