Skip to content

Conversation

@UndeadDevel
Copy link
Contributor

This partially fixes #1537, but while the increased width wouldn't be a problem on the NV41 AFAICT, I don't know about other machines.

I don't know what @tlaurion means with "busybox's folding", which may be a better solution.

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 1, 2023

@UndeadDevel

fold is a normally shipped Linux command which permits to spread output on multiple lines when displayed to user.

You can try it on from shell command line on common Linux OSes by default.

String="This is a really too long string of too many characters that won't display on a single line to be shown on a single line and needs to be folded to be showed correctly without hacking around"

echo "$String" | fold

Fold accepts parameters, which are different from busybox. Drop on recovery shell and try fold --help, but busybox replicates standard usage without parameters.

https://linux.die.net/man/1/fold

Standard width is 80 characters which is pretty standard and will work with older 1076 width screens and should not need to be worked around.

We cannot imply fixed width under Heads console or whiptail UX.

@UndeadDevel
Copy link
Contributor Author

Thanks for the clarification! Changed to fold-based solution now; tested in recovery shell of NK Heads 2.1.

@JonathonHall-Purism
Copy link
Collaborator

Thanks for checking this out!


I think this needs echo -e just before fold, i.e. echo -e "$passphrases" | fold <...>. Otherwise fold could break the \n and it will break later lines incorrectly not realizing there was a line break escape (it would think it is one giant line). The '\n' is handled by whiptail currently.

Adding echo -e might compound problems if passphrases themselves contain \ (now they'll be incorrectly substituted twice) but that is already broken unfortunately, so I think echo -e is OK to make this mergeable.

I didn't test any of it, that's what it looks like from review, please correct me if I missed something 😉


IMO, I don't think we need to fold to WIDTH-20 (=60) columns, I think we can use all 80 columns. With 60 columns, some reasonable-length passphrases will word wrap unnecessarily.

We're spending 35 chars on LUKS Disk Recovery Key passphrase: , at 60 that leaves just 25 for the passphrase before it wraps. Most 4-word passphrases with no separators are longer than that. At width 80 we'd have 45 chars.

UndeadDevel and others added 3 commits December 6, 2023 14:52
This partially fixes linuxboot#1537, but while the increased width wouldn't be a problem on the NV41 AFAICT, I don't know about other machines.

I don't know what @tlaurion means with "busybox's folding", which may be a better solution.

Signed-off-by: Christian Foerster <christian.foerster@mailfence.com>
Uses fold on the entire passphrase string now; tested in recovery shell of NK Heads 2.1.
Reverted change of WIDTH parameter (first commit of this PR).

Signed-off-by: Christian Foerster <christian.foerster@mailfence.com>
Signed-off-by: Christian Foerster <christian.foerster@mailfence.com>
@UndeadDevel UndeadDevel force-pushed the UndeadDevel-patch-secrets-window-width branch from d246b18 to ce2abd4 Compare December 6, 2023 13:53
@UndeadDevel
Copy link
Contributor Author

@JonathonHall-Purism Thank you for the feedback! You are right about grep needing -e; I missed that in testing at first; it's not just about backslash, but also ; chars. Regarding the WIDTH, however, I found that using 80 still hides some characters, because whiptail includes a margin around the edges, within which characters are invisible, but after some more experimentation in recovery shell I found that subtracting 5 from WIDTH will fix this issue, i.e. use all available (visible) space but not miss any characters.

I also rebased and signed off now.

Copy link
Collaborator

@JonathonHall-Purism JonathonHall-Purism left a comment

Choose a reason for hiding this comment

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

Looks great to me! @tlaurion any objections?

@tlaurion
Copy link
Collaborator

tlaurion commented Dec 7, 2023

Calculations seem right, cannot test but should be improvement nonetheless. Merging

@tlaurion tlaurion merged commit 851cc7f into linuxboot:master Dec 7, 2023
@UndeadDevel UndeadDevel deleted the UndeadDevel-patch-secrets-window-width branch December 7, 2023 15:50
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.

Summary window of all secrets after OEM factory reset doesn't display long passphrases in full

3 participants