Bug 510545

Summary: RFE: encryption key escrow support
Product: [Fedora] Fedora Reporter: Miloslav Trmač <mitr>
Component: anacondaAssignee: Anaconda Maintenance Team <anaconda-maint-list>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: anaconda-maint-list, awilliam, dlehman, rmaximo, vanmeeuwen+fedora
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-10-30 11:33:26 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 508954, 508963    
Bug Blocks: 508960, 531407    
Description Flags
Code snippets and some demonstration code
Patches against upstream git repo, adding escrow support
Patches against upstream git repo, adding escrow support
Patches against upstream git repo, adding escrow support none

Description Miloslav Trmač 2009-07-09 14:50:38 EDT
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.

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
Comment 1 Miloslav Trmač 2009-07-23 19:45:20 EDT
volume_key is now available in rawhide.
Comment 2 Miloslav Trmač 2009-08-10 13:46:59 EDT
Created attachment 356927 [details]
Patches against upstream git repo, adding escrow support
Comment 3 Chris Lumens 2009-08-28 14:07:08 EDT
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.
Comment 4 Miloslav Trmač 2009-09-05 05:27:43 EDT
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.
Comment 5 David Lehman 2009-09-08 14:58:50 EDT
Why not integrate the writing of escrow packets into Storage.write?
Comment 6 Miloslav Trmač 2009-09-08 15:10:27 EDT
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.
Comment 7 Chris Lumens 2009-09-08 15:49:54 EDT
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.
Comment 8 Chris Lumens 2009-09-08 15:52:02 EDT
Scratch that - we need /etc/fstab and /etc/mtab before mkinitrd runs, and that happens during package installation.
Comment 9 David Lehman 2009-09-08 19:16:18 EDT
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.
Comment 10 Miloslav Trmač 2009-09-12 22:12:05 EDT
Created attachment 360827 [details]
Patches against upstream git repo, adding escrow support

Thanks for the review, here's an update.

- Drop unused .encryptedDevice assignments
- Move writeEscrowPackets inside doPostInstall
- Fix bugs introduced while moving code to storage.formats.LUKS
Comment 11 David Lehman 2009-09-14 17:32:42 EDT
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
- 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.
Comment 12 Miloslav Trmač 2009-09-14 17:57:38 EDT
(In reply to comment #11)
> Your patches went into the anaconda git repository today

> - 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.
Comment 13 Huzaifa S. Sidhpurwala 2009-10-28 01:29:13 EDT
Any comment on the above.
This is causing kickstarts attempting to encrypt pvs to break.
See bz #531407
Comment 14 Huzaifa S. Sidhpurwala 2009-10-28 01:34:35 EDT
Any idea why the Comment 12 was not included in the anaconda patch?
Comment 15 David Lehman 2009-10-30 11:32:18 EDT
(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.
Comment 16 Adam Williamson 2009-10-30 11:33:26 EDT
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