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

initializer/cryptsetup: rework bash entrypoint #1140

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

jmxnzo
Copy link
Contributor

@jmxnzo jmxnzo commented Jan 10, 2025

Similarly to #1161 , this PR serves as a preliminary step toward moving cryptsetup to a subcommand of the initializer binary. Before translating the Bash entrypoint (used in the initializer binary after merging #1161) into Go code, the script was reworked to allow a cleaner integration of the volume encryption.
There were a few peculiarities in the cryptsetup LUKS standardization that have not been fully addressed in the current version of the entrypoint:

  • key is equivalent to passphrase (see the LUKS EXTENSION section in the cryptsetup manual).
  • When using the --key-file flag with cryptsetup, the file's contents are unexpectedly treated as a passphrase rather than a cryptographic key.
  • Using the keyfile as passphrase won't result in higher entropy of the encryption key. As a result, we can directly use the workload secret and UUID without performing key derivation, because there is no straightforward way to avoid the built-in PBKDF for LUKS in cryptsetup.

e2e/volumestatefulset: https://github.com/edgelesssys/contrast/actions/runs/12764474415/job/35576714135

@jmxnzo jmxnzo added the no changelog PRs not listed in the release notes label Jan 10, 2025
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch from 8c985ab to 5ad4898 Compare January 13, 2025 13:21
@jmxnzo jmxnzo changed the base branch from main to cryptsetup/initializer/basic January 14, 2025 09:46
Base automatically changed from cryptsetup/initializer/basic to main January 14, 2025 11:51
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch 2 times, most recently from 06f59f8 to eddd6bc Compare January 16, 2025 15:38
@jmxnzo jmxnzo changed the base branch from main to cryptsetup/initializer/basic January 16, 2025 15:40
@jmxnzo jmxnzo marked this pull request as ready for review January 16, 2025 15:40
@jmxnzo jmxnzo changed the title initializer.cryptsetup: rework bash entrypoint initializer/cryptsetup: rework bash entrypoint Jan 16, 2025
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch 2 times, most recently from 5e7c79c to ec4f47e Compare January 16, 2025 15:48
@jmxnzo jmxnzo requested a review from burgerdev January 17, 2025 14:12
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/basic branch from 7a57a5c to 492ee9b Compare January 20, 2025 07:36
@jmxnzo jmxnzo requested a review from katexochen as a code owner January 20, 2025 07:36
Base automatically changed from cryptsetup/initializer/basic to main January 20, 2025 07:54
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch from ec4f47e to c176341 Compare January 20, 2025 09:12
Copy link
Contributor

@burgerdev burgerdev left a comment

Choose a reason for hiding this comment

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

lgtm, but I'd like to get a second opinion from @3u13r.

internal/kuberesource/parts.go Outdated Show resolved Hide resolved
@burgerdev burgerdev requested a review from 3u13r January 20, 2025 10:42
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch 4 times, most recently from 7253c27 to 01c9cfb Compare January 21, 2025 11:19
@jmxnzo
Copy link
Contributor Author

jmxnzo commented Jan 21, 2025

As described in Figure 4: pseudo code for key creation each PBKDF2 derived encryption key used to encrypt the AFSplit of the master key, is derived by the passphrase and a 32 byte salt from a random source.
For a key slot, all parameters how to decrypt its key material with a given user password are
stored in the phdr (f.e. salt, iteration depth) with the corresponding encrypted master key stripes. Thus the random salt fulfills the same function as including the UUID into our passphrase and similarly adds the uniqueness of the disk to the encryption key. Therefore to maintain readability, we dropped the logic of doing the first key initialization to get the UUID and rekeying afterwards. @3u13r Please give feedback on this code change again!

internal/kuberesource/parts.go Outdated Show resolved Hide resolved
internal/kuberesource/parts.go Outdated Show resolved Hide resolved
@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch from 01c9cfb to c9f2576 Compare January 21, 2025 11:58
@jmxnzo jmxnzo requested review from 3u13r and burgerdev January 21, 2025 12:01
Copy link
Member

@3u13r 3u13r left a comment

Choose a reason for hiding this comment

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

LGTM

@jmxnzo jmxnzo force-pushed the cryptsetup/initializer/rework-bash branch from c9f2576 to b6eb084 Compare January 22, 2025 07:41
@jmxnzo jmxnzo merged commit 82cc928 into main Jan 22, 2025
10 checks passed
@jmxnzo jmxnzo deleted the cryptsetup/initializer/rework-bash branch January 22, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PRs not listed in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants