Skip to content

Add interface to change LUKS iteration time#3634

Merged
svartkanin merged 13 commits intoarchlinux:masterfrom
haouarihk:master
Jun 30, 2025
Merged

Add interface to change LUKS iteration time#3634
svartkanin merged 13 commits intoarchlinux:masterfrom
haouarihk:master

Conversation

@haouarihk
Copy link
Copy Markdown
Contributor

@haouarihk haouarihk commented Jun 27, 2025

PR Description:

Added a user interface to configure the LUKS encryption iteration time through the interactive menu system.

Why:

I found myself always changing this value after performing archinstall.
because i can't bother to wait 10 seconds for my disk to decrypt.
i would much prefer if its 1-3 seconds.
i know this can be a security issue for some people out there.
so i opted into adding an interface for users to change it instead.

Changes:

  • Added "Iteration time" option in the encryption menu
  • Added input validation (100-120000ms range)
  • Added translation support for the new interface (just the base.pot)
  • Updated encryption configuration to pass iteration time to LUKS

Affects:

  • LUKS encryption on partitions
  • LVM on LUKS encryption
  • LUKS on LVM encryption

Default: 10000ms (10 seconds).

Note: This PR follows the existing code patterns and maintains consistency with the current architecture. All changes are properly typed, documented, and include appropriate error handling.

Tests and Checks

  • I have tested the code!
  • Tested everything default value(10000ms), seems to slowly decrypt (expected behavior)
  • Tested changing iteration time to 100ms, it seems to quickly decrypt

@haouarihk haouarihk requested a review from Torxed as a code owner June 27, 2025 13:29
Comment thread archinstall/lib/disk/encryption_menu.py Outdated
except ValueError:
return tr('Please enter a valid number')

try:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this try block necessary? I don't see similar code in other parts of the codebase.

Some of these imports are already included at the top of the file, too, so I'm not sure why they would fail here.

Copy link
Copy Markdown
Member

@Torxed Torxed left a comment

Choose a reason for hiding this comment

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

@svartkanin if you're happy with the menu logic, then I'm happy with how this looks.

Copy link
Copy Markdown
Contributor

@correctmost correctmost left a comment

Choose a reason for hiding this comment

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

Two comments above and some issues I noticed while testing things locally:

  1. ruff check and flake8 warn about whitespace errors
  2. There seems to be a bug with the error handling:
    • Set the iteration time to 10ms
    • Press enter to get an "Iteration time must be at least 100ms" error
    • Press enter again to get an "Iteration time cannot be empty" error even though 10000 is present in the edit field

Comment thread archinstall/lib/disk/encryption_menu.py Outdated
),
MenuItem(
text=tr('Iteration time'),
action=lambda x: select_iteration_time(x),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like the lambda wrapping can be removed:

$ ruff check --preview
archinstall/lib/disk/encryption_menu.py:70:12: PLW0108 Lambda may be unnecessary; consider inlining inner function

https://docs.astral.sh/ruff/rules/unnecessary-lambda/


ENC_IDENTIFIER = 'ainst'

DEFAULT_ITER_TIME = 10000
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be good to use the new const in archinstall/lib/luks.py too:

iter_time: int = 10000,

@haouarihk
Copy link
Copy Markdown
Contributor Author

haouarihk commented Jun 27, 2025

Thanks for the feedback.

i haven't tested it locally. because i couldn't get the dev enviroment to work.
i'm just waiting for the iso file here to test it.

Edit: i also ran ruff check and flake8 and fixed the spacing warning

@correctmost
Copy link
Copy Markdown
Contributor

i haven't tested it locally. because i couldn't get the dev enviroment to work.
i'm just waiting for the iso file here to test it.

You can try this until the workflows are approved (I don't have approval permissions):

cd /root/
git clone --branch master https://github.com/haouarihk/archinstall.git
cd archinstall

# Repeat these steps whenever you make any code changes locally
rm -rf dist/
uv build --no-build-isolation --wheel
uv pip install dist/*.whl --break-system-packages --system --no-build --no-deps

# Launch your custom-built archinstall
archinstall

I still see some warnings when running ruff check --preview. And there's still some bugginess after an invalid value like 10ms is set, so that part needs more testing.

@Torxed
Copy link
Copy Markdown
Member

Torxed commented Jun 27, 2025

Ah right, new contributors require runners to be approved! They should be running now :)

@haouarihk
Copy link
Copy Markdown
Contributor Author

@correctmost Thanks soo much. that's very useful. and i got the dev enviroment to work

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i've been testing for quite a while.
and i found this to solve my issue with input persistancy.

there could be better solutions out there.
but i couldn't get _current_text to persist with the previewed value.

i also tested it in the other inputs and it seems to not effect them.

@svartkanin
Copy link
Copy Markdown
Collaborator

svartkanin commented Jun 28, 2025

The preview of the new iteration is missing on the outer menu
image

Other than that looks good 👍

@haouarihk
Copy link
Copy Markdown
Contributor Author

@Torxed i've tested the compiled iso. and it seems to be working as expected.
whenever you get a time, take a look at it.
i wont be making any more modifications unless i'm told to do so.

Thanks

@svartkanin svartkanin merged commit b3b00aa into archlinux:master Jun 30, 2025
9 checks passed
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.

4 participants