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

FIPS compliance #51

Open
elmarco opened this issue May 2, 2019 · 93 comments
Open

FIPS compliance #51

elmarco opened this issue May 2, 2019 · 93 comments

Comments

@elmarco
Copy link
Contributor

elmarco commented May 2, 2019

There are some concerns regarding implementation of crypto functions in libtpms in order to get FIPS compliancy.

Most of the crypto functions in libtpms come from the specification
https://trustedcomputinggroup.org/specifications-public-review/

and use openssl.

However, some crypto algorithm are open-coded, or use outdated openssl functions where better alternatives exist.

We need to identify those functions and provide alternatives when possible.
Since the code comes from the specification, we will have to discuss how the code changes can be integrated to the specification, before or after they are changed in libtpms etc.

@elmarco
Copy link
Contributor Author

elmarco commented May 2, 2019

From IRC discussion with Stefan:

10:21 <stefan_berger> some of the crypt algorithms (like key generation) were probably opened up so they can be cancelled, which is a features stemming from the hw interface and is available via sysfs
10:22 <stefan_berger> though I don't support cancellation in swtpm (but only in the CUSE implementation)

@stefanberger
Copy link
Owner

To avoid using those opened-up crypto algorithms that keep on reading the cancel flag we could introduce a compile time flag like TPM_NO_COMMAND_CANCEL and use functions that rely on calling into OpenSSL functions. I suppose this would address at least some of your concerns.

@stefanberger
Copy link
Owner

stefanberger commented May 2, 2019

Unfortunately it's not that simple with keys that need to be regenerateable from a template. They are derived from a random number generator based on a seed and a key derivation function (KDF). For those the call into OpenSSL wouldn't work since those keys need to be re-generateable. For keys that can be truly random a call into OpenSSL would work. CryptCreateObject() is the function that takes the rng as parameter.

@simo5
Copy link

simo5 commented May 2, 2019

@stefanberger hi, I asked @elmarco to look into some of this.
Our concerns are indeed open implemenation of cryptographic algorithms like RSA, HMAC, ECDSA, etc.. we'd like to be able to replace all of them with the equivalent validated cryptographic functions provided by OpenSSL. Key generation and KDFs themselves also falls within the purview of FIPS so would need to use OpenSSL provided functionality.
What KDF do you use that you see problems with ?

@stefanberger
Copy link
Owner

Keys themselves can also have seeds and these keys can be migrated from one TPM to another and always have to generate the same child keys using their seeds. So the function testing for prime numbers has to behave exactly the same way (on different vendor's implementations) so that the child keys can be repeatably generated. So OpenSSL RSA key generation function could be used as long as it produces always the same keys as the existing algorithm does.

@stefanberger
Copy link
Owner

@simo5 KDFa seems to be involved with symmetric keys and KDFe with ECC keys.

@simo5
Copy link

simo5 commented May 2, 2019

Looks like these KDFs have been implemnted in accord with the recommendations put forth in
NIST Special Publication 800-108, so they should be fine.
OpenSSL carries a number of KDFs already, and none matches thse we can propose new additions upstream.

I assume your previous comment about migration means these KDFs are set in stone and not changeable?

@simo5
Copy link

simo5 commented May 2, 2019

In fact, at least CryptKDFe looks like is an implementation of The Concatenation KDF decribed in SP 800 56 (I implemented this KDF in python cryptography a while ago and sounded familiar :-)

@stefanberger
Copy link
Owner

I think that's what key creation depends on and it needs to be repeatable for the same seeds using a KDF. The KDF will have to be the same then to get the same keys. From what I am hearing RSA keys cannot be derived from migrated keys.

@simo5
Copy link

simo5 commented May 2, 2019

Yes KDFs are deterministic, it's their purpose to be.
What do you mean with "RSA keys cannot be derived from migrated keys" ?

@stefanberger
Copy link
Owner

"NOTE 3 TPM2_CreateLoaded() can be used for creation of asymmetric keys but it may not be used for derivation of certain types of asymmetric keys. This limitation is because of the variability in algorithms for some asymmetric key types (such as RSA). " My guess it may be related to the prime number generation or testing that the could be slightly different and lead to different RSA keys.

@stefanberger
Copy link
Owner

Though we may have the constraint that if a user creates a derived RSA key with the current implementation and then upgrades the TPM 2 implementation to the FIPS one that derived RSA key would still have to be the same as with the old implementation.

@simo5
Copy link

simo5 commented May 2, 2019

From the FIPS point of view, this is not a problem, as you cannot "upgrade" to FIPS.
Keys that are not created in FIPS mode cannot be used anyway, so inability to migrate keys between FIPS and non-FIPS mode would be "ok".

@stefanberger
Copy link
Owner

stefanberger commented May 2, 2019

I looked at the HMAC functionality and how it could be converted. I think the trouble lies in a function like TPM2_HMAC_Start() that would create an OpenSSL HMAC_CTX. Unfortunately this context is opaque so that it would prevent vTPM state suspend and resume because we wouldn't be able to take the HMAC state (it has pointers!) and resume it later on to finish the HMAC with TPM2_SequenceComplete.

@simo5
Copy link

simo5 commented May 2, 2019

Yes, all the new OpenSSL API is opaque and uses pointers. And there are no functions to export state.
How critical is this?

@stefanberger
Copy link
Owner

This depends on the use case of your environment. If you want to upgrade your host without having to shut down all VMs then you may either want to suspend the VMs or migrate them to another host. So in this case it would matter. It would of course be possible to maintain internal counters about how many OpenSSL contexts are open and prevent suspend only while there are some open, which may again enable suspend/resume.

@simo5
Copy link

simo5 commented May 2, 2019

Understood.
We may start with a lock for open HMAC sessions, and then see if upstream openssl is open to add an HMAC state export function. But perhaps simply delaying suspend until the HMAC is done is sufficient. It is unlikely that large/lengthy HMAC operations are being done via TPM anyway.

@stefanberger
Copy link
Owner

... gives a lot of control to users to prevent migration though ...

@simo5
Copy link

simo5 commented May 2, 2019

yeah, we'll definitely have to deal with it.

@stefanberger
Copy link
Owner

Also CMAC would need functions for exporting and importing of context.

@stefanberger
Copy link
Owner

I started a branch now where we could do this work. My first step was to convert symmetric encryption/decryption using AES. https://github.com/stefanberger/libtpms/tree/pure_openssl

@simo5
Copy link

simo5 commented May 3, 2019

Absolutely fantastic, looks like a clean job so far.

@stefanberger
Copy link
Owner

AES and TDES should be done.

@stefanberger
Copy link
Owner

I hope that FIPs allows setting the RNG of OpenSSL because this is the only way we could get deterministic RSA keys derived from a KDF via an RNG. I converted all the other operations of RSA to use OpenSSL functions, so being allowed to do this is critical.

@simo5
Copy link

simo5 commented May 8, 2019

It's probably a grey area at this point, but I will look into it once I am back from Red Hat Summit.

@stefanberger
Copy link
Owner

I have now prototyped the conversion of AES, TDES, HMAC, CMAC, and RSA functionality to use OpenSSL APIs. Left to do is EC related code. The hashing code already uses the OpenSSL library. Here's the 'fallout' from this prototyping so far:

  • AES: no consequences
  • TDES: no consequences
  • HMAC: missing APIs in OpenSSL to support suspend/resume. Incompatibility with state that's being stored by existing code (breaks upgrade path)
  • CMAC: Same as HMAC
  • RSA: I haven't tested this yet, but the deterministically created keys in the existing code could very well be different than those created by OpenSSL (breaks upgrade path).

So, my biggest worry here is the upgrade path. One solution may be to use different compile time options for different distros... :-(

@simo5
Copy link

simo5 commented May 10, 2019

So the real breaker for upgrade seems to be mostly the RSA key derivation.
HMAC and CMAC are just suspend/resume issues as far as I can see, but not necessarily upgrade issues (unless you upgrade while the machine is suspended and there was an HMAC/CMAC operation in flight).

@stefanberger
Copy link
Owner

Yes, RSA key derivation breaks it.

'Operation in flight' is the occasion when an HMAC and CMAC sequences may have started before a VM suspend and would be expected to be completed after the resume, though because of the upgrade they either cannot resume or return the wrong result.

@stefanberger
Copy link
Owner

One possibility could be to introduce some flag that indicates whether a seed was created under the new code and if so uses the new key creation method, otherwise uses the old one... not nice for the code, though.

@simo5
Copy link

simo5 commented May 10, 2019

So I am looking at the code, and I am not sure there is value in trying to use OpenSSL's function to derive RSA keys.
Using a non CSPRNG random number generator is not going to be FIPS compliant anyway.

@stefanberger
Copy link
Owner

One of the issues is whether we can create all necessary key types in OpenSSL while deriving them from a KDF. Playing around with OpenSSL's RNG for this purpose may not be allowed...

@simo5
Copy link

simo5 commented Jun 10, 2019

It is allowed to use a DRBG in this manner, but we need to use the correct CSPRNG.

CCing Tomáš, he knows the code better than I around OpenSSL RNGs

@t8m can you help ?

@stefanberger
Copy link
Owner

Small patch for TDES key creation when no KDF is used: #68

@t8m
Copy link

t8m commented Jun 11, 2019

What is the requirement for the KDF? Why not use some preexisting KDF to derive the keys? I.e. do not use a fixed seed to DRBG but instead use a regular KDF such as HKDF.

@simo5
Copy link

simo5 commented Jun 11, 2019

@t8m, keys are derived from the seed each time the "device" is "restarted", the intention was to find out if we can be compatible with previous code, so that existing seeds keep "working" (ie generate the same keys as before).

@t8m
Copy link

t8m commented Jun 11, 2019

But what was the previous code doing to derive the keys? Did it somehow depend on the exact workings of OpenSSL rng? Or did it use its own DRBG implementation to derive the keys? Or?

@stefanberger
Copy link
Owner

The original TPM 2 code use an RNG backed by a KDF (rather than /dev/urandom) to generate the keys. The original TPM 2 code creates primes itself, tested them, etc. so that no OpenSSL functions other than BIGNUM related ones were used.

@t8m
Copy link

t8m commented Jun 13, 2019

I am afraid that here we are out of ways that could be possibly FIPS compliant. The only current FIPS approved method for generating keys based on a preselected seed that we implement in OpenSSL is the DSA FIPS 186-3 based keygen. But that does not help you. We could ask a FIPS test lab what is their opinion about generating RSA/ECDSA keys by some similar method. But then anyway it would have to be implemented in the OpenSSL directly and not outside of it (otherwise the libtpms would have to be inside a FIPS module boundary).

@simo5
Copy link

simo5 commented Jun 13, 2019

@t8m baby steps, let's get as close as we can, then we'll make a summary of what may still not be compliant and we'll see what can be done either by more breaking changes enabled only in FIPS mode or by opening a conversation with NIST via our Lab on these key derivation issues.
I have to assume the key derivation itself is something that would be interesting to NIST because a lot of actual TPMs must work that way in order to conserve memory.

@t8m
Copy link

t8m commented Jun 13, 2019

Is it possible they do not use key derivation but key wrapping? I.e. the RSA/ECDSA key is generated randomly but wrapped with a key stored in TPM.

@stefanberger
Copy link
Owner

No, they use key derivation in many cases, such as for the root keys of hierarchies (TPM2_CreatePrimary) as well as other keys that are child keys (TPM2_CreateLoaded).

https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p55_Part3_Commands_code_pubrev.pdf

@stefanberger
Copy link
Owner

But then anyway it would have to be implemented in the OpenSSL directly and not outside of it (otherwise the libtpms would have to be inside a FIPS module boundary).

This sounds like OpenSSL's crypto library will have to support the KDFs that TPM 2 defines as well as provide APIs to derive any type of key (RSA, EC, AES, Camellia, TDES) from those KDFs, since this is what TPM 2 also supports. I hope that the OpenSSL community is receptive to such an extension...

@simo5
Copy link

simo5 commented Jun 17, 2019

@stefanberger I think we should worry about perfect compliance later, and for now get as closer as we can get to using a crypto module (OpenSSL) so that we greately reduce the "gap" between what is compliant and waht is not.
As for KDFs and RNGs NIST has a specific set of allowed algorithms, so we'll have to look into it more carefully once we figure what can be used from OpenSSL, what can't, and what creates issues (eg, HMAC and saving state, or the KDF).
There is always an option that companies may decide to certify libtmps or a vTPM component as a cryptographic module of its own, in that case some cryptography (like the KDF) can be implemented in the library, but of course still offloaing as much as possible to OpenSSL will greately reduce the scope of work (and cost).

So for now I would simply note the issues, and work on low hanging fruits and make a summary once we know all problematic corners so that a plan can be developed on how to best address those.

@stefanberger
Copy link
Owner

@simo5 I think the pending PRs get us as to the first major hurlde of what can be done with libcrypto without having to sacrifice functionality in libtpms, such as for example state suspend/resume support, which is the reason why I am not including HMAC/CMAC in such PRs.

@stefanberger
Copy link
Owner

I revised the RSA PR a bit: #65 Unless there are comments I would merge by the end of the week. There are some performance losses due to having to recreate the private exponent D all the time, but this could be addressed separately if performance was a concern.

@simo5
Copy link

simo5 commented Jun 19, 2019

Sooo rsaKey does not include D and needs to be recomputed every time ?

@stefanberger
Copy link
Owner

We haven't stored D so far because the original TPM 2 algorithms didn't need it and used the Chinese Remainder Theorem (CRT) parameters for private keys and its operations, which seems to speed things up. OpenSSL unfortunately also needs the D parameter, which could be a bug, but it doesn't help since we would crash if we don't provide it...

@stefanberger
Copy link
Owner

PR #71 now implements support for the two opposite key operations: decryption and signature verification

@simo5
Copy link

simo5 commented Jun 19, 2019

We haven't stored D so far because the original TPM 2 algorithms didn't need it and used the Chinese Remainder Theorem (CRT) parameters for private keys and its operations, which seems to speed things up. OpenSSL unfortunately also needs the D parameter, which could be a bug, but it doesn't help since we would crash if we don't provide it...

Uhmm indeed it could be argued that if the CRT parameters are provided then d shouldn't be necessary, openssl uses the crt as well iirc ... anyway I understand why you need to add it now which is what I needed.

@stefanberger
Copy link
Owner

PR #72 now uses OpenSSL for creating purely random keys (if rand == NULL) just like it does for TDES and EC keys.

@stefanberger
Copy link
Owner

I believe the merged PRs reflect now all the changes that can be done with OpenSSL at the moment. So the biggest issue now is:

  • key derivation cannot be cleanly done with OpenSSL for the key types (RSA, EC, AES, TDES, etc.) and (custom) KDFs supported by TPM 2

@simo5
Copy link

simo5 commented Jun 20, 2019

@stefanberger yes I think this was a very nice piece of work, thanks a lot for it.
For the next steps I am planning to talk with our certification lab when I have time to see what are viable options wrt KDF, saving state, etc...

@stefanberger
Copy link
Owner

Ok.

FYI: The next patch series is an upgrade to revision 155 of the TPM 2 specs (PR https://github.com/stefanberger/libtapms/pull/70). I may have to delay this to rev 156 or so, though. There were some issues with RSA key generation on 64 bit machines, which is fixed there. To take this code and to maintain backwards compatibility I have to start tracking when seeds are generated and use the new code for new seeds while dragging along the old key generation code to keep on generating the same keys... The same seed tracking needs to then be done for switching to OpenSSL key generation, if this was to happen.

@stefanberger
Copy link
Owner

@simo5 Is there a requirement on what OpenSSL crypto APIs an application is supposed to or must use to (easier) become FIPs compliant? Like one should/must only use EVP_* functions?

@simo5
Copy link

simo5 commented Jul 2, 2019

In general EVP is strongly preferred where possible.

@nmav
Copy link

nmav commented Jul 8, 2019

@stefanberger I see that what you are describing in terms of RSA key generation, is very close to the Shawe-Taylor algorithm. That's a FIPS approved algorithm (FIPS186-4), and generates RSA keys from a seed (and is additionally a provable key generation method). gnutls uses this algorithm internally when provable keys are requested (e.g, certtool --generate-privkey --provable --seed "123456" will generate always the same key). I do not know whether openssl supports this algorithm though.

@t8m
Copy link

t8m commented Jul 8, 2019

Unfortunately openssl does not implement this algorithm.

@elmarco
Copy link
Contributor Author

elmarco commented Aug 1, 2019

Is there a todo-list of things known left to work on? (sorry I didn't read the whole thread)

@stefanberger
Copy link
Owner

Afaik I did everything that can be done for changing TPM 2 code to use OpenSSL functions.

@stefanberger
Copy link
Owner

Is there a todo-list of things known left to work on? (sorry I didn't read the whole thread)

TCG published a FIPS 140-3 guidance document: https://trustedcomputinggroup.org/resource/tcg-fips-140-3-guidance-for-tpm-2-0/

There's an old document here: https://trustedcomputinggroup.org/resource/tcg-fips-140-2-guidance-for-tpm-2-0/

Known issues:

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

No branches or pull requests

5 participants