Created attachment 351118 [details] Code snippets and some demonstration code Please add support for storing LUKS encryption keys to anaconda. I'm afraid rawhide anaconda does not currently work, which does not allow me to prepare a complete patch; attached is mostly tested code with some blanks, and some stubs to run on an ordinary Fedora system. Overview of the feature ----------------------- The user can, in a kickstart file (only), add "--escrowcert=URL:/to/x.509/certificate" to the "autopart" command or one of the "part, logvol, raid" commands. This will cause the encryption key used on all created volumes (or on the specified volume) to be encrypted using the specified certificate and saved in /root of the installed system. In addition, if --backuppassphrase is specified, a random passphrase is generated for each volume, added to the LUKS volume, and again saved in /root of the installed system. Required software ----------------- Adding the support will require adding volume_key-python and volume_key-libs to the installation image. volume_key requires NSS initialization; this can be done explicitly by adding python-nss to the installation image as well, or by using an ugly hack - rpm transaction set creation initializes NSS as a byproduct. Provided code ------------- Walking through it by "sections" separated using the ^L character: * Basic imports. Can be used as-is, except for NSS_NODB_HACK. As described above, anaconda can either add one more dependency, or use a hack. The code provides both options. * volume_key-python stub. Until the volume_key-python package is available, you can use this code as a minimal replacement to test the rest of the code against. Possible enhancement: check that the block device path passed to VKVolume.open() can be opened for reading and writing. * "External" stubs. Used for running parts of anaconda as non-root on an ordinary Fedora. You won't need this. * "Internal" stubs. Simulates the environment in the "anaconda" executable. You won't need this either. * Certificate download. Used to download the certificates from the URLs specified using --escrowcert. Each URL is dowloaded only once. "loadEscrowKeys" should be added to the "dispatch" list sometime after %pre is processed (so that it can e.g. set up local certificates from here documents, making the kickstart file self-contained), and probably before starting to copy packages. "loadEscrowKeys" is idempotent, running it more than once doesn't hurt. * Escrow. Does the actual work on LUKS volumes and $rootPath/root. "escrowPackets" should be added to the "dispatch" list in the post-install phase, probably before setFileCons,definitely before runPostScripts. I'm afraid I couldn't run anaconda and see how the storage code works: the function needs to determine affected volumes (if autopart --escrowcert was used, all created LUKS volumes; otherwise, the named volumes with --escrowcert), and run "escrowVolume" with correct parameters for each volume. An alternative is to split escrowVolume and this step into two: 1. perform the operations on LUKS volumes, storing the created packets in memory. This can be done before package installation, after all LUKS volumes are created and all LUKS passphrases are known (i.e. after "storagedone" at least). 2. create the files in $rootPath/root, in the post-install phase. * A test "main" program. You won't need this. Testing ------- Given a suitable kickstart file in "./kickstart-full", /dev/loop0 set up as a LUKS volume with passphrase "my passphrase", and a certificate in "/home/mitr/a/cert.pem", you can run "python scaffolding.py" to test the code outside of Anaconda. This tests kickstart parsing, download of the certificates specified by --escrowcert in the kickstart file, and working on LUKS (using hard-coded paths and passphrases). Other necesssary changes ------------------------ * Make sure the "loadEscrowKeys" and "escrowPackets" dispatch steps are enabled in a kickstart installation. * Add volume_key-{libs,python} files, and optionally python-nss files, to the installation image
volume_key is now available in rawhide.
Created attachment 356927 [details] Patches against upstream git repo, adding escrow support
Patch #4 is the big one, so that's what I'll comment on most. The others are really just support. (1) I'm fine with pulling in python-nss and getting rid of the hack. Why wouldn't we want to include python-nss? (2) Why did you choose to implement this as separate dispatcher steps, rather than incorporating it within kickstart.py? I'd rather see the certs downloaded on an as-referenced basis from within kickstart.py. True that adds a fair bit to kickstart, but it puts the download and load of the cert in the same place as it's referenced, which should help us to follow the flow of things in the future. (3) writeEscrowPackets could probably similarly be worked into the various write methods in the storage modules, which could eliminate that as a separate dispatcher step too. (4) I don't like the insider knowledge that the EscrowVolume class has, especially not of the passphrase. It seems like if we could better integrate these methods with the existing storage classes, we wouldn't need this class at all.
Created attachment 359862 [details] Patches against upstream git repo, adding escrow support OK, here is a new version. The certificate loading was integrated into kickstart directive processing, and data is stored either directly in formats.LUKS objects, or in anaconda.id.storage (for "autopart"). "autopart" escrow data is propagated into all automatically created LUKS partitions during autopartitioning. Writing the escrow packets post-installation remains a separate dispatch step, but most of the code was integrated into formats.LUKS.
Why not integrate the writing of escrow packets into Storage.write?
AFAICS that happens before packages are installed, i.e. before /root is created. It seems more natural to me to do it in the post-install phase, together with other code that writes to /root.
I don't see any reason why Storage.write needs to be called there. It looks like you should be able to move it into InstallData.write and then proceed with the integration.
Scratch that - we need /etc/fstab and /etc/mtab before mkinitrd runs, and that happens during package installation.
It's getting very close now. It would be better to call writeEscrowPackets from AnacondaBackend.doPostInstall (backend.py) to avoid creating a new dispatcher step for what is, in most cases, a user-invisible task. The only other thing I see is "encryptedDevice" attributes still being set in kickstart.py when they are no longer needed AFAICT.
Created attachment 360827 [details] Patches against upstream git repo, adding escrow support Thanks for the review, here's an update. Changes: - Drop unused .encryptedDevice assignments - Move writeEscrowPackets inside doPostInstall - Fix bugs introduced while moving code to storage.formats.LUKS
Your patches went into the anaconda git repository today with the following changes: - Remove hunk that changes kickstart.Partition to check for pd.onPart instead of pd.preexist (all other device types use preexist) - Don't pass escrow args to lvmpv format constructor in storage.partitioning._scheduleLVs - Move backup passphrase generation into storage.devicelibs.crypto - Use newer, clearer except syntax in storage.writeEscrowPackets They will be included in anaconda-12.25-1.
(In reply to comment #11) > Your patches went into the anaconda git repository today Thanks! > - Remove hunk that changes kickstart.Partition to check for pd.onPart > instead of pd.preexist (all other device types use preexist) AFAICS pykickstart.commands.partition.*_PartData only parses onPart, not preexist. See also other parts of kickstart.Partition.parse.
Any comment on the above. This is causing kickstarts attempting to encrypt pvs to break. See bz #531407
Any idea why the Comment 12 was not included in the anaconda patch?
(In reply to comment #14) > Any idea why the Comment 12 was not included in the anaconda patch? It was left out for the reason stated in comment 12 (all other device types use preexist, not onPart). We did not want to change the partition handler without changing the other device handlers, and we were not sure whether or not it was necessary.
See 531407 comments #2 and #3: the code has been added as of anaconda 12.41. closing this report, please follow on at 531407. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers