tpm2-software / tpm2-tss-engine

OpenSSL Engine for TPM2 devices
https://tpm2-software.github.io
BSD 3-Clause "New" or "Revised" License
148 stars 97 forks source link

Asked for password when none required #13

Closed dwmw2 closed 5 years ago

dwmw2 commented 5 years ago

I created a fake X.509 cert to go with a key generated with tpm2tss-genkey. Now the openssl s_client invocation like the one in #12 works fine, except of course that I can't actually authenticate to my VPN server. It does offer the fake client cert correctly though.

However, I'm asked for a password even though I didn't specify one when creating the key.

AndreasFuchsTPM commented 5 years ago

Yes, that is correct. The TPM actually has no notion of "unset password" but merely of empty password. So this engine exposes exactly that behavior.

dwmw2 commented 5 years ago

Can we not try with an empty password and prompt only if that doesn't work?

The storage format of openssl_tpm2_engine includes an indication that the password is empty.

AndreasFuchsTPM commented 5 years ago

Change required here: https://github.com/tpm2-software/tpm2-tss-engine/blob/master/src/tpm2-tss-engine.c#L174

dwmw2 commented 5 years ago

You'll note you now have the 'emptyAuth' hint from the PEM file itself, but I haven't wired that up for you. (Neither have I wired up the 'parent' field, except to reject anything but RH_OWNER for now).

dwmw2 commented 5 years ago

For reference, see also the various request_passphrase() calls in my code in http://git.infradead.org/users/dwmw2/openconnect.git/blob/tpm2:/gnutls_tpm2_esys.c where we ask (repeatedly, if needs be) for passwords for the hierarchy, the parent key, and the wrapped key itself.

Although the header there says LGPLv2.1 it's all my code (except the bits I copied from you) and you're welcome to use it under the 3-clause BSD licence if any of it is useful, or if you just want to be careful about pollution from looking.

AndreasFuchsTPM commented 5 years ago

I noticed, just did not want for this to be forgotten. I have an ENGINE_CTRL for ownerpass, thus I don't want to call the user repeatedly, but thanks!

dwmw2 commented 5 years ago

You can't rely on ENGINE_CTRL; the calling application might not know the special details of your ENGINE. If we fix the STORE thing then they might not even know they're using an ENGINE.

As things stand, if an owner password is set you can't even use these keys with the openssl command line, can you? Various things like s_client, s_server, req work with ENGINE keys... but only where the UI callbacks are used.

dwmw2 commented 5 years ago

Likewise, OpenConnect will load the engine but will expect UI callbacks if the engine wants anything; it doesn't make special arrangements (and how could it? It doesn't even know you have a password on the hierarchy).

AndreasFuchsTPM commented 5 years ago

I guess you are right aboutth hierarchy. We can do this. But only RH_OWNER is allways NODA=1

Problem: If a parent is specified in the PEM and this parent has NODA=0, then "trying" an empty password first will increment DAcounter. DAcounter increments are the last thing on earth you'd want... And since no-password is the default for storage keys, asking allways would be hidious (as you pointed out for auth-less keys).

Given this, do we maybe want to add a "parent-noauth" flag to the PEM-format ?

dwmw2 commented 5 years ago

Given this, do we maybe want to add a "parent-noauth" flag to the PEM-format ?

That seems like a sane suggestion. @jejb?

dwmw2 commented 5 years ago

For persistent parents I do already check if they have the NODA flag and prompt for a password if they don't. (At least, my IBMTSS implementation does this.)

Can we do the same for hierarchy auth, and actually tell at runtime if they have NODA?

AndreasFuchsTPM commented 5 years ago

The hierarchy auth allways has NODA, if I remember correctly. So we can allways just try emptyauth first for RH_OWNER.

dwmw2 commented 5 years ago

OK, so we don't need the parent-noauth flag?

FWIW James's engine and my code support NULL, ENDORSEMENT and PLATFORM hierarchies too. Not sure why those four specifically; I'm mostly just pretending to have a clue here...

dwmw2 commented 5 years ago

Fixed in my own tpm2-esys code to check the NODA flag and demand a password without trying the empty one first: http://git.infradead.org/users/dwmw2/openconnect.git/commitdiff/cc8289826ab5c744c350e5e547865710c1e1faa9

jejb commented 5 years ago

On Fri, 2018-10-12 at 09:31 -0700, David Woodhouse wrote:

For persistent parents I do already check if they have the NODA flag and prompt for a password if they don't. (At least, my IBMTSS implementation does this.)

Unfortunately NODA isn't a sufficient indicator. It just means don't engage the TPM password attack lock mechanism, not I have well known authority. There are genuine reasons why you'd do a NODA key with a password.

The provisioning standard specifically says the storage hierarchy root key should have a well known authorization.

It's also the original openssl_tpm2_engine position that the person running the implementation should understand the platform requirements around the parent key; that's why the parent auth is an engine parameter. Once we do keys in the Endorsement hierarchy, a simple password prompt won't cut it: the standard mandates they be accessed with a policy which is going to be a huge headache (and not really amenable to a prompt)

Can we do the same for hierarchy auth, and actually tell at runtime if they have NODA?

I think the architecture guide does say that permanent handles (which includes the primary seeds) don't have DA protection.

James

dwmw2 commented 5 years ago

Unfortunately NODA isn't a sufficient indicator. It just means don't engage the TPM password attack lock mechanism, not I have well known authority. There are genuine reasons why you'd do a NODA key with a password.

That's fine though. We cope with a NODA key with a password. The point in NODA is that it doesn't matter if we try an auth with an empty password... so we do, and then only prompt for a password if that fails.

If NODA isn't set, then we prompt for a password in advance without ever trying an empty password.

dwmw2 commented 5 years ago

It's also the original openssl_tpm2_engine position that the person running the implementation should understand the platform requirements around the parent key; that's why the parent auth is an engine parameter.

Er, what? Applications use OpenSSL STORE and know absolutely nothing about TPMs. The user just passes them a PEM file. This ENGINE happens to be installed in the system, and loading the PEM file works. What is it, that you are asking every application to know about and do for you?

UI callbacks are the only thing you can rely on in the general case.

dwmw2 commented 5 years ago

Or they know just enough to invoke the engine, but that's it. For example

openssl s_client -engine tpm2 -keyform engine -key tpmkey.pem -cert cert.pem -connect $DEST:443

Hell, I can't even use the openssl command line tool to generate a CSR for a key in the TPM, unless the UI callbacks work, can I?

AndreasFuchsTPM commented 5 years ago

So James's view:

My thought and David's view:

The problem with the former is of course the convenience of the STORE interface and "dump" applications. The problem with the latter view is that a password could be set at a later point and an extra flag (it should be ignored for RH_OWNER). The fact that it does not cover endorsement auth policies: It helps with RH_OWNER, and does not prevent anything in endorsement, so no losses here.

I'm in favor of the parent-emptyauth indicator, since I'd also love to see the STORE interface and "dump" applications in use... @jejb Can you live with that extra flag ?

jejb commented 5 years ago

On Fri, 2018-10-12 at 12:44 -0700, Andreas Fuchs wrote:

So James's view:

  • If a user has no idea, they will create a key under RH_OWNER with empty password (or even callback if necessary)
  • If a user has enough idea to use their own parent, they have enough idea to set the password in advance (optimizable by trying empty password first, if noda is set)

My thought and David's view:

  • If a user has no idea, they will create a key under RH_OWNER with empty password (or even callback if necessary)
  • Even a user with enough idea to use their own parent cannot set it, because the application is not intelligent enough, so we add a parent-emptyauth indicator to the PEM.

I thought David's idea was to predicate on NODA. If it's set we can try empty auth and then ask if that fails, so no need for a password flag. If NODA is 0 we assume the reason is because auth is provided so ask all the time.

James

dwmw2 commented 5 years ago

Yeah, both my ibmtss and tpm2-tss back ends are now checking a persistent parent key for NODA, and prompting for its password in advance if NODA isn't set. If NODA is set we try empty auth for the parent first. That covers persistent parent keys.

For the generated primaries, didn't James say that the hierarchies shouldn't have DA protection? So trying empty auth first (as I do) is also fine there? And both engines need to learn to call the UI there, since that's currently failing for OpenConnect's OpenSSL build.

AndreasFuchsTPM commented 5 years ago

For Primaries, we allways try emptyauth and then ask the user.

I'm talking about the following process for non-primary parents:

Agree or not worth it ? You say not worth it ?

jejb commented 5 years ago

On Fri, 2018-10-12 at 13:57 -0700, Andreas Fuchs wrote:

For Primaries, we allways try emptyauth and then ask the user.

I'm talking about the following process for non-primary parents:

So we're clear: the only possible non-primary parents are NV keys because we don't have the machinery in the file to specify all the parents up to the primary.

  • If NoDA is set: Try emptyauth
  • If NoDA is clear && parent-emptyauth is set: Try emptyauth

This one simply assumes we need a password

  • Else ask for password. The second step is only possible if we add parent-emptyauth (beyond David's current approach).

Agree or not worth it ?

What would be the reason for clearing NODA and yet having a well known password?

James

dwmw2 commented 5 years ago

What would be the reason for clearing NODA and yet having a well known password?

No good reason that I can imagine. Although I can't see how to get the tpm2-tss tpm2_createprimary tool to create a key without NODA set, regardless of whether the password is empty or not. Which means I'm currently being prompted for the (empty) password for PEM files parented by that particular primary in my testing.

So now I do think I understand why Andreas still wants this 'parent-emptyauth' flag. But screw it, I'd rather stick with "Don't Do That Then" and fix tpm2_createprimary to set NODA automatically when there's no password.

AndreasFuchsTPM commented 5 years ago

Ok, agreed...

dwmw2 commented 5 years ago

OK then... tpm2-tss-engine is already updated to the new ASN.1 form and 'TSS2 PRIVATE KEY'. The OpenConnect tpm2 branch reads the same keys and also the 'TSS2 KEY BLOB' from openssl_tpm2_engine... for which I have sent patches to read and output the new form, while still loading the old for compatibility.

We know that policy and importable keys are left for future development, so are we OK to do releases and write I-Ds based on what we have now? Do we need an official assignment of the OIDs, given that I seem to have conceded we'll continue to use them?

AndreasFuchsTPM commented 5 years ago

Would be nice to have this, together with the format, for other people attempting to use TSS2 PRIVATE KEYs in their applications...

AndreasFuchsTPM commented 5 years ago

@dwmw2 @jejb I just remembered, why I don't first try "emptyAuth" if NODA=set:

Only the loadkey method has a UI parameter for talking to the user. The sign and decrypt functions don't. Thus we cannot ask from there.

So the only possibility would be to go for some "test-operation" within loadkey, which IMHO is not really worthy the effort.

Opinions and comments welcome !?!

dwmw2 commented 5 years ago

Only the loadkey method has a UI parameter for talking to the user. The sign and decrypt functions don't. Thus we cannot ask from there.

That can be fixed.

AndreasFuchsTPM commented 5 years ago

That would make a lot of sense... Are you picking this up ?

dwmw2 commented 5 years ago

I am happy to facilitate communication and try to get everyone agreeing on file formats and user experiences... but if I can avoid having to actually implement it myself all for all three variants, and provide test cases for everyone too, then I'll be happier. I really am going to have to go back to real work soon, especially now that my GnuTLS code in OpenConnect is doing the RightThing™.

AndreasFuchsTPM commented 5 years ago

:+1: I'll worry about this engine...

safayetahmedatge commented 5 years ago

One concern is the possibility of fully unattended operation (IOT applications). In such scenarios, prompting the user for any information is not required or even acceptable. The tool will be stuck waiting for user input forever, because there is no user.

In this case, the desired behavior would be to try an "emptyAuth", and fail if that fails (no prompting the user for passwords).

One possibility would be to affect password behaviour at build time: use an ifdef to replace the body of "get_auth" with something that statically sets an empty password. For example, openssl makes use of the "OPENSSL_NO_UI" preprocessor flag (see "password_callback" in apps/apps.c in the openssl source).

dwmw2 commented 5 years ago

That's up to the application. If it wants to register a UI method that doesn't prompt the user but just unconditionally returns failure, that's fine.

safayetahmedatge commented 5 years ago

Thanks for the response, @dwmw2. I apologize for any misunderstanding but I didn't mean a custom IOT application written in C. I meant using the existing openssl tools with the tpm2tss engine for an IOT use case. Is it possible to control the default UI behavior of openssl utilities (through the environment or config files for example)?

AndreasFuchsTPM commented 5 years ago

@safayetahmedatge you can use the -passin parameter. -passin stdin for example takes auth from a pipe. -passin pass:abc will pass "abc" to the engine. I think you'll be able to accomplish what you want (not being stuck on user-prompt) using this parameter.

Also if you create the key with the (to be implemented) "empty-auth" parameter, then the engine will also never ask for a password...

safayetahmedatge commented 5 years ago

Thanks @AndreasFuchsSIT. I will look into the empty-auth parameter once it's implemented (let me know if I can help).

Just for future reference, I tried to use the passin argument but that didn't work. Openssl tools req and x509 with the tpm2tss engine prompted for a password even when an appropriate pass argument was provided.

From what I see in the openssl source, I don't think it can work. Specifically, I don't see a clean way for an engine to get access to the password argument passed to openssl utilities.

The actual parsing and handling for the passin and passout options is done in the app_passwd function in apps/apps.c.

The password obtained by app_passwd is passed to the load_key function in apps/apps.c. The load_key function stores a pointer to the password in a local PW_CB_DATA structure and passes a pointer to this structure to the engine key-loading function, ENGINE_load_private_key (in crypto/engine/eng_pkey.c).

However, the PW_CB_DATA structure is cast as a void pointer. Further, the engine is not really passed a call-back function to make use of this data structure. I don't see a way for any engine to get to the password from app_passwd.

The only engine implementation in openssl that appears to implement load_privkey and make use of passwords is for nCipher Cryptographic Hardware Interface Library (CHIL) in engines/e_chli.c. It's not entirely clear to me what's going on in that code.

jejb commented 5 years ago

On Wed, 2018-10-17 at 20:31 -0700, safayetahmedatge wrote:

From what I see in the openssl source, I don't think it can work. Specifically, I don't see a clean way for an engine to get access to the password argument passed to openssl utilities.

It definitely works if you use the correct UI process flow. If you look at the tests for the openssl_tpm2_engine, you can see it being used:

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tests

Here's a simple test creating and verifying a self signed ECC cert with a TPM resident key with a password:

jejb@jarvis:~/tmp/tmp> create_tpm2_key -a -k passw0rd --ecc prime256v1 tmp.key jejb@jarvis:~/tmp/tmp> openssl req -new -x509 -passin pass:passw0rd -subj '/CN=test/' -key key.tpm -engine tpm2 -keyform engine -out tmp.key engine "tpm2" set. jejb@jarvis:~/tmp/tmp> openssl verify -CAfile tmp.crt -check_ss_sig tmp.crt tmp.crt: OK

openssl works in a fairly convoluted way: the cb_data passed to the engine load private key has to be passed back into the UI with UI_add_user_data() before you call UI_process(). If you do this correctly, you won't get a prompt.

James

AndreasFuchsTPM commented 5 years ago

35 fixes most of the issues in here. Outstanding is only that OpenSSL does not provide a UI parameter to sign and decrypt functions, which is why we cannot test emptyAuth on noda.

I'll close this ticket, until openssl's api changes, which may take a few years...

safayetahmedatge commented 5 years ago

@AndreasFuchsSIT thanks for the update. It works now without prompting for a password.

@jejb I can confirm that the tools and engines behave as expected with the "pass" parameter. When a non-empty password is provided through "passin", the user is not prompted for a password.

However, it appears that the "passin" mechanism can't handle empty passwords. All of the following resulted in a password prompt:

In any case, the tools and engine are behaving as desired now with the no_auth changes. Thanks everyone.

jejb commented 5 years ago

On Thu, 2018-10-18 at 15:17 +0000, Andreas Fuchs wrote:

35 fixes most of the issues in here. Outstanding is only that

OpenSSL does not provide a UI parameter to sign and decrypt functions, which is why we cannot test emptyAuth on noda.

I'll close this ticket, until openssl's api changes, which may take a few years...

Actually, you can, it just depends whether you want to. The way I tell in engine load key whether the key password is correct is to try to get a bound session on the NODA key. That will fail very fast if you have the wrong authorization and doesn't require running trial crypto operations.

James

AndreasFuchsTPM commented 5 years ago

Makes sense... @jejb do you keep the session around for the actual crypto operation or do you just flush it afterwards ?

jejb commented 5 years ago

On Thu, 2018-10-18 at 10:11 -0700, Andreas Fuchs wrote:

Makes sense... @jejb do you keep the session around for the actual crypto operation or do you just flush it afterwards ?

I actually use it to load the public part of the key, but then it's flushed immediately after and the former is only because of a quirk in the IBM TSS, so I'd just bind the session, treat an error return as wrong password and then flush.

The current in-kernel resource manager doesn't re-gap the TPM so all session use needs to be as short as possible.

James

AndreasFuchsTPM commented 5 years ago

Ok, sounds like something I could add as well... :+1:

@jejb P.S. you as pointed out here: https://lists.gt.net/linux/kernel/2596683#p2597845 (sorry I had to do this.....)

AndreasFuchsTPM commented 5 years ago

Implementation note: During LoadKey of a persistent key, if attribute NODA is set, StartAuthSession with bind to key We can ignore non-persistent keys because the carry the empty-auth bit within the PEM file.

AndreasFuchsTPM commented 5 years ago

@jejb I just looked into implementing this procedure here as well, but one thing irritates me: During a StartAuthSession, not authorization is required. Instead, the key's authValue goes into the calculation of the created session's key. Thus an error on StartAuthSession cannot indicate an authorization failure.

What part did I miss about your proposed approach ? Thanks!

jejb commented 5 years ago

On Fri, 2018-11-23 at 01:12 -0800, Andreas Fuchs wrote:

@jejb I just looked into implementing this procedure here as well, but one thing irritates me: During a StartAuthSession, not authorization is required. Instead, the key's authValue goes into the calculation of the created session's key. Thus an error on StartAuthSession cannot indicate an authorization failure.

What part did I miss about your proposed approach ?

We use the bound session for TPM2_ReadPublic() of the key with an encrypted return for good measure. If that fails, the auth is wrong.

James