Skip to content
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

Make attest-enroll more configurable, pluggable; make it escrow and make it use AEAD #125

Merged
merged 20 commits into from
Jul 12, 2021

Conversation

nicowilliams
Copy link
Contributor

@nicowilliams nicowilliams commented Jun 13, 2021

Tested manually.

This PR adds:

  • re-enroll and enroll-more options
  • pluggable/configurable generators of contents for an enrollment -- this will be used to provision an enrolled host with: a signature key and certificate, a host keytab, and maybe other things
  • authenticated symmetric encryption of secrets
  • escrow to zero, one, or more bare RSA public keys
  • escrow to zero, one, or more TPMs (with optional policy)
  • TK or EK method of secrets transport to enrolled host, with optional, configurable policy
  • configuration file (though everything has a command-line option as well)
  • index by hostname

TBD:

  • add commit hook so we can do git add/git commit/git push, or whatever, at the end of enrollment
  • add a test (which should start multiple swTPMs, some for escrow, some for enrollees, and which should test secret recovery by enrollee and by escrowers, policies, etc.)

Current usage message:

Usage: attest-enroll [OPTIONS] HOSTNAME < ekpub
       attest-enroll [OPTIONS] -I EKPUB HOSTNAME

  Options:

    -h          This message
    -v          Verbose
    -x          Trace

    -a          Add to existing enrollment of this EKpub
    -r          Replace the current enrollment of this EKpub
    -I EKPUB    EKpub file (default: read from stdin)

  Configuration options:

    -C CONF     Config file (default: /etc/safeboot-enroll.conf)
    -E DIR      Directory containing PEM files containing escrow keys.
                These should be named "tpm-<name>.pub" and contain an
                escrow TPM's EKpub, or "<name>.pem" and contain an RSA
                key for PKCS encryption.
    -G GENPROG  A program to generate a secret and/or metadata (details below).
                May be given multiple times.  Hostname (public) and filesystem
                keys (secret) are always generated first.
    -O DIR      Output directory (default: /safeboot/sbin/build/attest)
    -P POLICY   A policyDigest or a script that outputs one.  The TPM to which
                the enrolled EKpub belongs _will_ enforce the policy when
                decrypting secrets for it.
                (default: )
    -T METHOD   Secrets transport method.  MEDTHOD is one of:
                 - TK (uses a long-lived RSA transport key whose primary key is
                       TPM2_Duplicate()ed to the enrolled EKpub)
                 - EK (uses TPM2_MakeCredential() to encrypt to the enrolled
                       EKpub)
                (default: EK)

  CONF is a bash script that should set/clear one or more of:

    DBDIR ESCROW_PUBS EKPUB POLICY ESCROW_POLICY TRANSPORT_METHOD
    GENPROGS (bash array)

  GENPROGS will be called in the order they are given, with the following
  arguments:

    TMP-OUTPUT-DIR ENROLL-DIR HOSTNAME

  and are expected to a) create a file in OUTPUT-DIR, b) output:

    sensitive NAME1 [NAME2 ..].

  or

    public NAME1 [NAME2 ..].

  or

    skip [REASON]

  E.g.,

    sensitive SOMEPRIVKEY

    sensitive SOMEPRIVKEY.PEM SOMEPUBKEY.PEM SOMECERT.PEM

    public SOME-META-DATA-HERE

    skip NOT NEEDED

  The NAMEs are of files in the TMP-OUTPUT-DIR.  Any sensitive NAME1 file will
  be encrypted and escrowed; all other files will be copied to the enrollment
  area for the given EKpub.

@nicowilliams nicowilliams force-pushed the pxe-server branch 5 times, most recently from 03eba1e to df67546 Compare June 15, 2021 20:01
@nicowilliams
Copy link
Contributor Author

Need to add tests.

@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jun 15, 2021

Here's a sample config:

: ; cat /etc/safeboot-enroll.conf
DBDIR=/tmp/o
ESCROW_PUBS=/tmp/o/e
TRANSPORT_METHOD=EK
GENPROGS=(genhostname genfskey gentest0 gentest1 gentest2 gentest3 gentest4 gentest5)
POLICY=
ESCROW_POLICY=
: ;

The gentestN geprogs are shell functions in attest-enroll to test GENPROGS/-G functionality.

Each genprog can:

  • do nothing (output skip or skip reason message here)
  • output zero or one secret file (to be encrypted to the enrolled device and to escrow keys)
  • output zero, one, or more public metadata files (not encrypted nor escrowed)
  • use files from the enrolled device directory

The idea is to allow multiple additive enrollments (when the -a option is given), and to make conflict resolution automatic.

Note that multiple hostnames are allowed, if you keep enrolling the same EKpub with different hostnames. If using a TK, the same TK will be used in all cases.

@nicowilliams
Copy link
Contributor Author

Maybe this script could also git add, git commit, and git push?

@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jun 15, 2021

TBD: Use authenticated symmetric encryption.

@nicowilliams nicowilliams force-pushed the pxe-server branch 2 times, most recently from eff54e9 to 881cb5c Compare June 15, 2021 21:38
@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jun 16, 2021

I think I'll get rid of the unique/dedup stuff. The genprogs can do that themselves.

@nicowilliams nicowilliams force-pushed the pxe-server branch 2 times, most recently from ecd7747 to 09a9256 Compare June 16, 2021 22:59
@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jun 16, 2021

Now with a test (which I'm running manually). I need to integrate it into the Mariner and non-Mariner workflows.

The test starts an swTPM for each of three simulated clients and an swTPM for an escrow agent, checks that all the clients can enroll, and then checks that tpm-receive.sh can be used to decrypt each client's secrets with the corresponding client's swTPM, and that the escrow agent swTPM can decrypt the escrowed secrets. The test also checks that each client cannot read the other client's secrets.

TBD:

  • run tpm2-attest and friends to check that the simulated clients can attest
  • check that each client can only decrypt its own secrets
  • test that policies work
  • add a genprog for issuing the clients certificates
  • add a genprog for issuing the clients keytabs
  • make sure tpm2-attest can tar up all the secrets, since there's more than one
  • make tpm2-attest encrypt/decrypt the tarball with aead_encrypt/aead_decrypt

@nicowilliams nicowilliams changed the title Pxe server Make attest-enroll more configurable, pluggable; make it escrow and make it use AEAD Jun 17, 2021
@nicowilliams
Copy link
Contributor Author

Not ready. I've got to finish changing various bits of various scripts.

@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jun 20, 2021

I may need to make each genprog do its own escrow and encryption to TPM. Why? Well:

  • it may not be possible to pass the other secrets to the post mount of / boot sequence (??);
  • genprogs could be invoked via sudo to get some privsep (even for rootfs.key, preventing compromise of the HTTP server from compromising new rootfs.keys would be useful);
  • second, so that we can have different policies for the rootfs.key and other things -- that way the policy for the rootfs.key can be that PCR 11 is unextended, and the policy for post early boot things can be that PCR 11 has been extended with a particular value.

Thoughts?

@nicowilliams nicowilliams force-pushed the pxe-server branch 2 times, most recently from 30f54dd to 182db39 Compare June 21, 2021 05:13
@nicowilliams
Copy link
Contributor Author

I now have a complete test -minus HTTP- that works. It tests attest-enroll, tpm2-attest quote, tpm2-attest verify, attest-verify, tpm2-attest seal, tpm2-attest unseal, and initramfs/bootscript (the last one with an env var that makes it exit after recovering the rootfs.key file).

I've also added TOFU golden PCR learning, which I needed for now, especially for testing.

I've removed uses of tar's -z, but I've not done s/\<tgz\>/tar/g yet.

TBD:

  • integrate with @geoffthorpe's flask and mariner build and test framework
    • there's a bit of work to do around enrollment
  • add support for different policies for earliest boot and later early boot (which we need)
    (the idea being that we could have the unextended-PCR#11 policy for rootfs.key and extended-once-PCR#11 policy for other secrets, that way we can attest twice, once during initramfs, and once after, and not have to worry about how to get other secrets from initramfs to post pivot)
  • add scripts for keytabs and such

@nicowilliams
Copy link
Contributor Author

Regarding privsep... the code that does the secrets generation, and the git add/commit/push, all has to be privsep'ed from the flask code (HTTP server) if we're going to do privsep at all, because otherwise a compromise of the HTTP server would allow the attacker to push known-to-them rootfs.keys for not-yet-installed systems, and then the attacker would know those systems' root keys.

@osresearch
Copy link
Owner

osresearch commented Jun 22, 2021

"77 files changed". This is going to take a little while to review...

For the most part the fixes for more modern bash look good, especially the ones around local variable scoping so that we don't have accidental scope creep.

Is the tar -zcf to tar -cf to remove a dependency on gzip or is there another reason that I'm missing? ah, I see you created an issue about it #127

The different policies for different keys makes lots of sense. That's something that we should figure out if there is a clean way to specify the alternate policy.

More later.

@osresearch
Copy link
Owner

Regarding the changes to die(), I agree that having a function inside a library call exit is not the best approach and can make error handling harder. Although having die() not exit means that the name is no longer as appropriate as the Perl-inspired die function. My suggestion is that we split this into five bash functions:

  • die: print a message to stderr and exit with an error code. This is only to be used in extreme emergencies.
  • error: print a message to stderr and return an error code. This is used by most functions when they encounter an error, replacing most of the current uses of die and often with the pattern: foo || error "error!" || return 1
  • warn: print a message to stderr when something isn't quite right, but we're going to keep on trying
  • info: print a message to stderr to update the user on progress (or invoke gnu texinfo, but nobody does that)
  • debug: print a message to stderr if a $DEBUG environment variable is set

@osresearch
Copy link
Owner

The house style is one hard-tab for each indentation and for multi-line shell commands to put the arguments one tab stop deeper, regardless of the length of the shell command so that changing commands doesn't require re-indenting the additional lines, with one argument and parameter per line, and with the line continuation characters one space separated from the end.

For tpm2 subcommands, I've been meaning to make sure that we're using long arguments everywhere to try to document their meaning as well. Not all the code has been updated to use this, I'm trying to do it consistently in the new code.

So instead of this:

	tpm2 loadexternal -C n -Gecc -r "${TMPDIR}/wkpriv.pem"	\
			  -L "${TMPDIR}/activate-policy.dat"	\
			  -a 'sign|adminwithpolicy'		\
			  -c "${TMPDIR}/wk.ctx"

The style guide would call for:

	tpm2 loadexternal \
		--hierarchy 'null' \
		--key-algorithm 'ecc' \
		--private "${TMPDIR}/wkpriv.pem" \
		--policy "${TMPDIR}/activate-policy.dat" \
		--attributes 'sign|adminwithpolicy' \
		--key-context "${TMPDIR}/wk.ctx"

@osresearch
Copy link
Owner

osresearch commented Jun 22, 2021

For sbin/attest-enroll, shellcheck has some concerns (I haven't reviewed them and some likely predate your changes; just pasting it in here):

In sbin/attest-enroll line 34:
. "$BASEDIR/../functions.sh"
  ^------------------------^ SC1090: Can't follow non-constant source. Use a directive to specify location.


In sbin/attest-enroll line 142:
	exit ${1:-1}
             ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	exit "${1:-1}"


In sbin/attest-enroll line 153:
G) GENPROGS+=($OPTARG);;
              ^-----^ SC2206: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.


In sbin/attest-enroll line 172:
[[ -n ${CONF} && -f ${CONF} ]] && . "${CONF}"
                                    ^-------^ SC1090: Can't follow non-constant source. Use a directive to specify location.


In sbin/attest-enroll line 206:
    ((VERBOSE)) && echo info: "$@" 1>&2 || true
                ^-- SC2015: Note that A && B || C is not if-then-else. C may run when A is true.


In sbin/attest-enroll line 252:
	rm -f $outdir/tk.pem
              ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	rm -f "$outdir"/tk.pem


In sbin/attest-enroll line 373:
ekhash="$(sha256sum $tmp/ek.pub | cut -f1 -d' ' )"
                    ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
ekhash="$(sha256sum "$tmp"/ek.pub | cut -f1 -d' ' )"


In sbin/attest-enroll line 377:
	hostname="$(echo $ekhash | cut -c1-8)"
                         ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean: 
	hostname="$(echo "$ekhash" | cut -c1-8)"


In sbin/attest-enroll line 404:
	set -- $("$i" "$tmp" "$tmp/ek.pub" "$hostname")
               ^-- SC2046: Quote this to prevent word splitting.

@osresearch
Copy link
Owner

One additional bit of flexibility that we should consider as part of the pluggable enrollment is that Windows requires specific per-machine file names for the encryption key. I've created a separate issue so we don't forget to think about it after this is merged #129

This adds aead_encrypt and aead_decrypt shell functions that implement
confounded and padded AES-256-CBC with HMAC-SHA-256 for integrity
protection.

AEAD == authenticated encryption w/ additional data

These utils are not quite AEAD because there's no AD support yet, though
it should be easy to add, being just more data thrown into the HMAC
computation input stream.
This:

	info() { ((VERBOSE>0)) && echo "$*"; }

breaks when VERBOSE == 0, causing `info` to return 1.

The issue is that there was no successful command in `info` when
VERBOSE == 0.

Rewrite as:

	info() { ((VERBOSE>0)) && echo "$*"; return 0; }
@nicowilliams
Copy link
Contributor Author

nicowilliams commented Jul 7, 2021

Just gotta make sure that tests/test-enroll.sh passes again.

EDIT: I've broken the send/receive scripts. Easy fix. (fixed). tests/test-enroll.sh passes again.

@nicowilliams
Copy link
Contributor Author

I think this is very close now.

Do not validate any PCRs in phase 1.

There's still code to TOFU learn golden PCRs for phase 2 systems...

But!  That's on the attestation server side, which has a read-only
DB.  How can that ever work?  It can't.  We need a different plan
for any "golden" PCRs handling.
@nicowilliams
Copy link
Contributor Author

This now also handles the PEM case using the fix from tpm2-tools#2780.

BTW, it isn't actually necessary for the TPM2B_PUBLIC of an EK to be correct for the purpose of encrypting to it. I tried it, and indeed, it doesn't matter if all the attributes or the policyDigest of the given EKpub match the actual one or not, as the cryptographic name of the target of TPM2_Duplicate() and TPM2_MakeCredential() is not bound in at all (unlike the cryptographic name of the activation object, which is bound in), so in fact we did not need tpm2-tools#2780, or the hack we had.

I do have a test in tests/test-enroll.sh that we do get the same EKpub when using PEM as when reading it from the TPM, but we don't actually need that to be the case in GCP and we don't need that to be the case on-prem, so we could just remove that check. To make that test possible I made sbin/attest-enroll fail in the case that an enrollment exists and there was no content to add to it -- this seems a bit silly. Maybe I should remove all that, as well as the dd-the-public-key-over-a-TPM2B_PUBLIC hack in pem2tpm2b_public(), since we don't need the hashes to match.

# Compute the policyDigest of a given policy by executing it in a trial
# session.
function make_policyDigest {
tpm2 flushcontext --transient-object
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tpm2_flushsome?

@osresearch
Copy link
Owner

  • rename new tools to match tpm2-*
  • yaml front matter for new docs
  • attest enroll text
  • nav page update for attestation
  • document ek policy (I think it is defined by the tcg)

 - tpm2-send encrypts a small secret to an EKpub using either of two
   encryption-to-TPM methods:

    - use TPM2_MakeCredential() with a well-known key's cryptographic
      name as the activation object's name,

    - use TPM2_Duplicate() to export a software RSA keypair to the EKpub
      and then encrypt the small secret to the RSA keypair.

   Both methods allow an optional policy to be asserted by the sender
   that the target TPM will enforce at decrypt time.

 - tpm2-recv decrypts ciphertexts made by tpm2-send

   Any policy asserted by the sender has to be provided for the purpose
   of executing and satisfying it.

The two methods have the same characteristics and are morally
equivalent:

 - all the sender needs is the target's EKpub,
 - only an entity with access to the target's TPM can decrypt
   ciphertexts addressed to that target
 - the sender gets to assert a policy that the target TPM will enforce
   when it comes time to decrypting the sender's ciphertext,

Credit:

 - Trammell Hudson came up with the TPM2_Duplicate() approach
 - Erik Larsson came up with the TPM2_MakeCredential() approach
Also: use the tpm2-send/tpm2-recv scripts.
@osresearch osresearch merged commit 574dad6 into osresearch:pxe-server Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants