Dave Lehman wrote:
* I'm a little unsure about adding the crypto dev to the fsset.Device
object. I wonder if instead it's cleaner to just integrate the crypto
code into the Device object. But I'm on the fence here I think
* If we go this route, NullCrypto is probably better than Passthrough
I started out just using the Device classes, but ended up up feeling
like it was the wrong way.
It seems logical to establish the passphrase at the same time we set up
the partition, and often we don't know the device/partition name at that
point (right?). Certainly we don't have a Device instance handy. I ended
up adding an encryption boolean and a passphrase to the RequestSpec
class, which I later passed to the Device constructor. So the Device
also has an encryption boolean and a passphrase, along with a bunch of
code that depends on the value of the boolean. It started to feel like a
hack. It may be cleaner in terms of overhead/minimalism, but it's way
dirtier in terms of design IMO.
That said, it may be that the more committed you/we are to only
supporting LUKS longer-term, the more acceptable it becomes to just
stuff it in where it fits in (the RequestSpec and Device classes).
I think that if we're to switch away from LUKS, it'll be a switch. Not
a matter of supporting multiple at a time. Maybe it makes sense to just
have a EncryptionInfo that contains all of the encryption related pieces
and then just do if that's None or not. But like I said, I'm a little
on the fence here.
* Multiple different types of crypto block devices seems like it's going
to end up being a UI nitemare. We should pick one path rather than
trying to support everything under the sun
Yes. I mainly wanted to lay the foundation so that extension is easier
if we choose to pursue it. I agree completely that we should only
support LUKS now, and anything beyond establishing a passphrase and a
crypttab entry is left as an exercise for the user.
The problem is if we leave the code such that it's there, someone is
going to want to extend it ;-)
* Is the filesystem.supportsEncryption attribute really needed? The
filesystem doesn't have to really support it at all as it's all done at
the block level
It's only there so the UI knows when to display the "Encrypt this
partition" checkbox. It could just as easily be a list in cryptodev.py.
I mainly want to be very clear about what we support, and I don't think
it should include crazy stuff like encrypted VGs.
But it's not based on the filesystem -- what wouldn't we show as being
able to encrypt? If it's just VGs, well, that's different code anyway
and we can just not include the checkbox on the VG editor
* The sanity checking should probably be combined a bit and likely in
partitions.sanityCheckAllRequests() as that's where we do other checks
for, eg, /boot not being on a PV
Makes sense. I basically tried to add the checks in the RequestSpec to
cover things like kickstart once we add support there, and to the UI to
prevent the user from ever thinking they can create, for example, an
encrypted /boot (nobody wants to think of a passphrase, enter it twice,
then get an error saying that encrypted /boot is not allowed).
sanityCheckAllRequests will get all the cases including kickstart. And
it's really the only right place. Otherwise, there's lots of "fun" in
ordering how you create /boot and /.
Jeremy
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
