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

initrdscripts: Make cryptsetup fail hard in unexpected conditions #3486

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

mtoman
Copy link
Contributor

@mtoman mtoman commented Jul 31, 2024

This patch adds a set of checks that make sure that the partition layout is as expected on a system with encrypted disks.


Contributor checklist

Reviewer Guidelines

  • When submitting a review, please pick:
    • 'Approve' if this change would be acceptable in the codebase (even if there are minor or cosmetic tweaks that could be improved).
    • 'Request Changes' if this change would not be acceptable in our codebase (e.g. bugs, changes that will make development harder in future, security/performance issues, etc).
    • 'Comment' if you don't feel you have enough information to decide either way (e.g. if you have major questions, or you don't understand the context of the change sufficiently to fully review yourself, but want to make a comment)

Copy link

github-actions bot commented Jul 31, 2024

Website deployed to CF Pages, 👀 preview link https://9b90b1a3.balena-os.pages.dev

return 0
}

cryptsetup_run() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll probably need to change this from always running, to always running when the device is in user mode. Otherwise, systems without secure boot may not boot.

@flowzone-app flowzone-app bot merged commit e086acb into master Sep 13, 2024
157 of 159 checks passed
@flowzone-app flowzone-app bot deleted the mtoman/cryptsetup-fail-hard branch September 13, 2024 16:21
@rettier
Copy link

rettier commented Sep 30, 2024

@mtoman we have seen issues with this change because lsblk sometimes seems to output an extra whitespace. here for example:

$ LUKS_PARTITIONS=$(lsblk -nlo "kname,uuid,fstype,partlabel" "/dev/${BOOT_DEVICE}" | grep "crypto_LUKS")
$ echo $LUKS_PARTITIONS
sda
sda1  3845-FC2                                                           vfat                balena-efi
sda2  0b587cd9-60bd-47dd-909e-f23e8f7b6e36 crypto_LUKS resin-boot
...

there are 2 spaces between sda2 and 0b...
when cutting the UUID later via cut -d " " -f2 a whitespace is actually extracted.

Other uses of lblk always cut the first column (-f1) thus were never affected by this.

I suggest adding tr -s " " in order to strip any duplicate whitespaces and make the behaviour consistent.

@mtoman
Copy link
Contributor Author

mtoman commented Sep 30, 2024

Thanks for the comment @rettier is this still the case with the latest master? We have observed this as well, shortly after merging the PR and c9544aa was supposed to fix it.

@rettier
Copy link

rettier commented Sep 30, 2024

@mtoman thanks for the info, i was looking through the latest merges but must have missed this one. It does indeed fix our issue.

Sorry for the unnecessary comment.

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.

3 participants