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
grub: add support for passwords #65231
Conversation
Works for me |
@0x4A6F please state what testing you have done and on what basis you are approving the PR. |
I've provisioned a system with
@JohnAZoidberg, if you could come up with other scenarios, I'm eager to test. |
2831e63
to
e2d1164
Compare
e2d1164
to
677bc4a
Compare
I have just tested with the following config. Of the 5 "boot.loader" lines, I tested them one at a time with the others commented out.
|
I tested every options and it's working as expected. The only issue I've found is that I guess if you |
ping |
Is there a good way to emit an error from this script during a build-vm event? |
I think what you are doing is just fine, the problem must be
It would be good to have a NixOS installer test for grub with passwords but I get the OCR to work in the grub screen. I'll try some more before giving up. |
So, I managed to get the output of grub to the serial console, which is easier to deal with. |
@tfc can you take a look at the test driver part? |
nixos/lib/test-driver/test-driver.py
Outdated
def process_serial_output() -> None: | ||
for _line in self.process.stdout: | ||
# Ignore undecodable bytes that may occur in boot menus | ||
line = _line.decode(errors="ignore").replace("\r", "").rstrip() | ||
try: | ||
self.last_lines.put(line, block=False) | ||
except queue.Full: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would rather see first that we really have the problem that some test fails because it goes OOM or gets too slow processing serial output, before we begin dealing with silently failing tests because we drop serial lines without further notice because a small 1000-line buffer overruns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't understand what you're asking here. Do you want this operation to be blocking or are you proposing to increase the buffer size (as above)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think that it would be a good idea to:
- make the buffer size infinite (get that with empty Queue constructor args)
- make the
put
calls blocking, assuming the queue never runs full - revisit the question if it is really necessary to model the notion of a 100-lines "screen", because it makes a difference for the caller if the "screen buffer" has 1, 10, or 100 screens stored currently. there is no way to get to the "last" screen. wouldn't it suffice to give
wait_for_console_text
semantics that are like "does the serial output match my regex since boot/i called this last time"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I removed the size limit and stored the lines received since wait_for_console_text
is called in a StringIO
.
This way we efficiently append lines into the buffer and can match multiline regexes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping @tfc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This patch adds support for user accounts/passwords in GRUB 2. When configured, everything but the default option is password-protected.
This method is similar to wait_for_text but is based on matching serial console lines instead of the VGA output.
Thank you for the review! |
This patch adds support for user accounts/passwords in GRUB 2.
When configured, everything but the default boot option in GRUB is password-protected.
Motivation for this change
This feature adds a small layer of physical security to NixOS systems.
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)