Page MenuHomeFreeBSD

Create the GELIBOOT GEOM_ELI flag
ClosedPublic

Authored by allanjude on Apr 6 2016, 9:32 PM.

Details

Summary

This flag indicates that the user wishes to use the GELIBOOT feature, to boot from a fully encrypted root file system.
Currently, GELIBOOT does not support key files, and when it does, they will be done a bit differently.

Due to the design of GELI, and the desire for secrecy, the GELI metadata does not know if key files are used or not, it just adds the key material (if it exists) to the HMAC before the optional user password, so there is no way to tell if a GELI partition has key files or not.
Since the GELIBOOT code in boot2 and the loader do not support keys, they will only attempt to attach if this flag is set

This will stop GELIBOOT from prompting for passwords to GELIs that it cannot decrypt, disrupting the boot process

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=208251

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

allanjude retitled this revision from to Create the GELIBOOT GEOM_ELI flag.Apr 6 2016, 9:32 PM
allanjude updated this object.
allanjude edited the test plan for this revision. (Show Details)
allanjude added reviewers: pjd, oshogbo, ed, gnn, emaste.
allanjude updated this revision to Diff 14950.
oshogbo edited edge metadata.Apr 6 2016, 10:46 PM

Small comment.

sbin/geom/class/eli/geom_eli.c
937 ↗(On Diff #14950)

md.md_flags & G_ELI_FLAG_GELIBOOT is not a boolean equation you should compare it. Ideal would be:
(md.md_flags & G_ELI_FLAG_GELIBOOT) == G_ELI_FLAG_GELIBOOT

Sometimes good enough version would be:
(md.md_flags & G_ELI_FLAG_GELIBOOT) != 0

940 ↗(On Diff #14950)

Same.

sys/geom/eli/g_eli_ctl.c
485 ↗(On Diff #14950)

same.

489 ↗(On Diff #14950)

same.

Ah I see now that all in the file is done the same comparison so not sure if you should change that.

wblock added a subscriber: wblock.Apr 6 2016, 10:59 PM
wblock added inline comments.
sbin/geom/class/eli/geli.8
297 ↗(On Diff #14950)

Couldn't this just be:

Enable booting from encrypted root filesystem.
298 ↗(On Diff #14950)

This does not seem necessary, and "unencrypted" combined with the earlier "encrypted" makes for a confusing logical "not".

299 ↗(On Diff #14950)

Simplify, passive -> active, and avoid "you" and "your":

The the boot loader prompts for the passphrase and loads
.Xr loader 8
from the encrypted partition.
496 ↗(On Diff #14950)

Repeating this identically here does not seem too helpful.

503 ↗(On Diff #14950)

Let's define what it does so the user does not need to know a new term:

Deactivate booting from an encrypted partition.

If you think it is worth it to fix these, I'll do it as a separate review after this one

allanjude marked 3 inline comments as done.Apr 6 2016, 11:31 PM
allanjude edited edge metadata.
allanjude updated this revision to Diff 14956.

Address manpage feedback from wblock

ed added inline comments.Apr 7 2016, 2:47 PM
sbin/geom/class/eli/geom_eli.c
1001 ↗(On Diff #14956)

It seems like we're proliferating tri-state options: yes, no and unspecified. Would it make sense to just come up with an enum somewhere at the top of this file and use those values instead of -1, 0 and 1?

allanjude added inline comments.Apr 7 2016, 3:45 PM
sbin/geom/class/eli/geom_eli.c
1001 ↗(On Diff #14956)

Probably a good idea. I can do this as a separate commit after.

oshogbo edited edge metadata.Apr 7 2016, 4:29 PM
oshogbo accepted this revision.
This revision is now accepted and ready to land.Apr 7 2016, 4:29 PM
ed edited edge metadata.Apr 7 2016, 4:30 PM
ed accepted this revision.
This revision was automatically updated to reflect the committed changes.