Skip to content

(feat): Enable full disk encryption #1634

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.

Sign up for GitHub

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

Jump to bottom
Open
vunnyso wants to merge 2 commits into
base: main
Choose a base branch
from
Open

(feat): Enable full disk encryption #1634

vunnyso wants to merge 2 commits into from
+125 −107

Conversation

@vunnyso
Copy link
Collaborator

@vunnyso vunnyso commented Dec 12, 2025 •
edited
Loading

Description of Changes

This change modifies the disk partitioning scheme to use LVM on LUKS, compared to the previous scheme which used LUKS on LVM. The advantages of the new scheme include reduced layout complexity and the ability to enable full disk encryption. With this approach, there is no need to encrypt different partitions separately.

Encryption

Disk encryption will be enabled for both debug and release images. To avoid entering PIN or password on every reboot. The PIN or password requirement applies exclusively to release images.

Authentication:

In debug builds, the boot process remains unchanged. However in release builds, users will be prompted to set a PIN or password during the first boot. For subsequent boots, they will need to enter the same PIN or password to unlock the system.

Partitioning layout details:

Encrypted area highlighted.

With this PR:
image

With this PR (if storage.encryption.enable = false):
image

With mainline:
image

Type of Change

  • New Feature
  • Bug Fix
  • Improvement / Refactor

Related Issues / Tickets

Checklist

  • Clear summary in PR description
  • Detailed and meaningful commit message(s)
  • Commits are logically organized and squashed if appropriate
  • Contribution guidelines followed
  • Ghaf documentation updated with the commit - https://tiiuae.github.io/ghaf/
  • Author has run make-checks and it passes
  • All automatic GitHub Action checks pass - see actions
  • Author has added reviewers and removed PR draft status

Testing Instructions

Applicable Targets

  • Orin AGX aarch64
  • Orin NX aarch64
  • Lenovo X1 x86_64
  • Dell Latitude x86_64
  • System 76 x86_64

Installation Method

  • Requires full re-installation
  • Can be updated with nixos-rebuild ... switch
  • Other:

Test Steps To Verify:

  1. Make sure Partitioning layout matches as mentioned in Description of Changes
  2. Please test with debug and release builds.

kajusnau
kajusnau reviewed Dec 12, 2025
View reviewed changes
Copy link
Collaborator

@kajusnau kajusnau left a comment

Choose a reason for hiding this comment

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

Great work!

Not my area of expertise so I'll leave proper review to others, just leaving a small suggestion. 🍻

@vunnyso
(feat): Enable full disk encryption
72b8f38 
This change modifies the disk partitioning scheme
to use `LVM on LUKS`, compared to the previous scheme
which used `LUKS on LVM`. The advantages of the new
scheme include reduced layout complexity and the
ability to enable full disk encryption. With this
approach, there is no need to encrypt different
partitions separately.

Signed-off-by: Vunny Sodhi <vunny.sodhi@unikie.com>
@vunnyso
docs: Add disk encryption documentation
f90a4e6 
Signed-off-by: Vunny Sodhi <vunny.sodhi@unikie.com>
@vunnyso vunnyso mentioned this pull request Dec 12, 2025
Draft
19 tasks
brianmcgillion
brianmcgillion approved these changes Dec 12, 2025
View reviewed changes
Copy link
Collaborator

@brianmcgillion brianmcgillion left a comment

Choose a reason for hiding this comment

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

Nice one!

@brianmcgillion
Copy link
Collaborator

brianmcgillion commented Dec 12, 2025

2 things though. you say in the PR description it is only for release but it is enabled in mvp-user-trial. so on all images. which is great actually. it should be on by default for all to get it tested.

Also you mention that it is a pin. is is a pin or a password?

@brianmcgillion brianmcgillion added the Needs Testing CI Team to pre-verify label Dec 12, 2025
@milva-unikie
Copy link

milva-unikie commented Dec 12, 2025

Are the laptop image sizes supposed to be 17–18 GiB?

@vunnyso
Copy link
Collaborator Author

vunnyso commented Dec 12, 2025

2 things though. you say in the PR description it is only for release but it is enabled in mvp-user-trial. so on all images. which is great actually. it should be on by default for all to get it tested.

Also you mention that it is a pin. is is a pin or a password?

Apologies for the confusion, disk encryption will be enabled for both debug and release images. To avoid entering PIN every reboot. The PIN requirement applies exclusively to release images.

I have tried it with release image. It accepts both PIN and password. (updated description)

@milva-unikie milva-unikie removed the Needs Testing CI Team to pre-verify label Dec 12, 2025
@milva-unikie
Copy link

milva-unikie commented Dec 12, 2025

(Vunny will check the image size, not ready for testing yet)

@vadika
Copy link
Contributor

vadika commented Dec 12, 2025

This needs to be refactored to be harmonized with A/B updates scheme. It need to use systemd-repart for partitioning!!!

@vadika vadika self-requested a review December 12, 2025 16:06
@brianmcgillion
Copy link
Collaborator

brianmcgillion commented Dec 12, 2025

This needs to be refactored to be harmonized with A/B updates scheme. It need to use systemd-repart for partitioning!!!

this is the debug image. not the repart image though. that is a different baseline in partitioning

@brianmcgillion brianmcgillion self-requested a review December 15, 2025 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kajusnau kajusnau kajusnau left review comments

@mbssrc mbssrc mbssrc approved these changes

@avnik avnik Awaiting requested review from avnik

@vadika vadika Awaiting requested review from vadika

@brianmcgillion brianmcgillion Awaiting requested review from brianmcgillion

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@vunnyso @brianmcgillion @milva-unikie @vadika @mbssrc @kajusnau