Closed danglus closed 6 years ago
Sorry @danglus, this one slipped through my filters. What you're asking for is something that belongs in a higher level API. I get your point: using the TPM for the HMAC calculation erodes any security that the use of the HMAC session. That said, the code in tpmclient
and the integration tests are meant to test the SAPI code so it's nice to get additional coverage this way. Also I'd prefer to keep from introducing an external dependency on a crypto library for these test cases.
IMHO the issue is that this code, that's really test code, is in a directory called samples
. To me this indicates that it's sample code that we provide for others to pick up and possibly use in their applications. Given the above, this is a bad idea.
I've been over working on infrastructure for the new tpm2-abrmd and neglecting the TSS repo a bit. But I'll be pivoting back here to clean up the test harness a bit and I'll include moving this test code to a location where it's intent is more clear. Getting a few comments in place describing what these functions are for will help too.
openssl has hmac code: https://www.openssl.org/docs/man1.0.2/crypto/hmac.html
like I said above: but pulling in a dependency on openssl just for our tests seems a bit heavy handed.
we need to clarify the intent: this function should have a huge honking warning in a comment that explains why it was written the way it was and that it's not sample / example code
alright, middle ground is to make the dependency on an external crypto library a "soft" dependency: it will only be required if the source tree is configured to build the test code. So the default case of just straight "./configure && make && make install" will not require openssl but a developer building and running test cases will need to "./configure --enable-test-unit --enable-test-integration && make && make check" and configure will check for the crypto lib
the sample code uses function pointers to call crypto routines. You can easily code a wrapper function around openssl or some other crypto library that matches the signature of the function pointer and then alter the function pointer to point to your new function.
Re the security properties of using the TPM itself to calculate the HMAC, it depends on what you're trying to defend against. I agree that it's less than optimal security, but also think that its better than it could be. It might be useful for very small footprint systems that can't contain a crypto library.
Will
I should also add, that I've done what I recommended above in my internal to Raytheon implementation. It works very well with openssl and another crypto library.
From: Philip Tricca [mailto:notifications@github.com] Sent: Wednesday, June 14, 2017 12:24 PM To: 01org/TPM2.0-TSS TPM2.0-TSS@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [01org/TPM2.0-TSS] A helper function for HMAC sessions is needed (#403)
Sorry @danglus https://github.com/danglus , this one slipped through my filters. What you're asking for is something that belongs in a higher level API. I get your point: using the TPM for the HMAC calculation erodes any security that the use of the HMAC session. That said, the code in tpmclient and the integration tests are meant to test the SAPI code so it's nice to get additional coverage this way. Also I'd prefer to keep from introducing an external dependency on a crypto library for these test cases.
IMHO the issue is that this code, that's really test code, is in a directory called samples. To me this indicates that it's sample code that we provide for others to pick up and possibly use in their applications. Given the above, this is a bad idea.
I've been over working on infrastructure for the new tpm2-abrmd
What’s this?
and neglecting the TSS repo a bit. But I'll be pivoting back here to clean up the test harness a bit and I'll include moving this test code to a location where it's intent is more clear. Getting a few comments in place describing what these functions are for will help too.
Where do you need help with comments?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/01org/TPM2.0-TSS/issues/403#issuecomment-308484290 , or mute the thread https://github.com/notifications/unsubscribe-auth/AEdleGGOW37eShdgTZuNk-47oG-3YOazks5sEAkNgaJpZM4NFumL . https://github.com/notifications/beacon/AEdleHiZiiNip15jNY0OhVa74OVWT9oXks5sEAkNgaJpZM4NFumL.gif
Here is a link to a standalone SHA-256 code (so a link to the crypto lab will not be needed). https://github.com/B-Con/crypto-algorithms/blob/master/sha256.c Can it be used instead?
@danglus You can use whatever you'd like. Unfortunately, we have to be careful about about how we use crypto and cannot speak to the quality of that body of work. We cannot endorse particular crypto technologies.
I understand what you say, but if someone is going to use HMAC authorization following the samples that are currently present (which use the TPM to calculate HMAC) the result will not have a security advantage over a simple password. For what it worth, I’ve been using this SHA-256 code for quite a while now, and the results has always been identical to OpenSSL.
@danglus Sample code tends to become production code. That sha256 implementation could be riddled with security issues like side channel attacks, etc.
A helper function for calculating HMAC authorization would really come handy. Currently, the tpmClient is using functions from /test/common/sample/SessionHmac.c which are using the TPM itself for calculating the authorization HMAC (with is, of course counter productive). A generic helper function should calculate the SHA256 locally (single file, open source sha256 implementations are available).