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

Clarify confusing reference to key identification blob as "private key". #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Beuc
Copy link
Contributor

@Beuc Beuc commented Mar 22, 2017

Hi,

I played around with my TPM using simple tpm-pk11, and I got quite confused by the mention of a "private key" being stored in "~/.simple-tpm-pk11/my.key" (what would be the point of using a TPM if the non-migratable private key is exported at generation time?).

After checking that TSS_TSPATTRIB_KEYBLOB_PRIVATE_KEY is indeed not used in stpm-keygen, I thought I would avoid headache for my peers through this patch :)

AFAICT the key blob is some kind of key identification but I didn't write that clearly since I couldn't find any official reference/definition about it (TrouSerS didn't help).

@ThomasHabets
Copy link
Owner

ThomasHabets commented Mar 23, 2017

Well, it is the private key. Encrypted with the SRK like it currently says. If it doesn't say "private key" then wouldn't that cause confusion since readers would ask "well, where's the private key, then?".

It is possible to store this blob on the limited TPM flash space, but this tool doesn't do that, partially because it's expected that the blob itself is stored on FDE, which adds a layer of security since it requires both the presence of the TPM in question AND that the FDE is unlocked. (and any key PINs)

@Beuc
Copy link
Contributor Author

Beuc commented Mar 23, 2017

Thanks for the clarification.

OK, actually what confused me what "SRK-encrypted" followed immediately by "By default the Well Known Secret (20 nulls) is used" and "The SRK password [...] is usually not required or useful".

So I understood that the SRK was null so it did not make sense it "encrypted" the private key.
Plus that's the only place where the blob was described as a private key (all the other manpages simply mention "key blob").

Now I see that there's a difference between "SRK" and "SRK password".

Maybe can we append "Note: the SRK password is different than the SRK itself, which is generated at TPM ownership."?

@ThomasHabets
Copy link
Owner

It is easy to misread, yes.

Maybe put this somewhere:

The "SRK password" is needed to be able to do operations with the "SRK", which is the actual cryptographic key. The user has no access to the SRK directly. The same goes for other keys protected by the TPM chip.

If so, where would you want this info to be written?

@ThomasHabets ThomasHabets self-assigned this Mar 23, 2017
@Beuc
Copy link
Contributor Author

Beuc commented Mar 23, 2017

IMHO:
Adding this in the README would be good (that's what I read first), e.g. in "Init TPM chip".
Additional clarification could explain the nature of the "key blob" since AFAICT this information is hard to get on the Internet.
man pages manipulating SRK password and/or key blobs could either clarify or point to the README.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@ThomasHabets
Copy link
Owner

Added my phrasing to the README.

@danni
Copy link

danni commented Jun 9, 2017

Hi so just to clarify, there's nothing stored in the TPM via this module? The generated key is wrapped with the TPM key and stored on the filesystem. Where is the generated key created? Inside the TPM?

Also it looks like you don't expose any mechanism other than CKM_RSA_PKCS? I assume this is enough for ssh and so that's where you stopped?

How does this module relate to tpmtoken_*

@ThomasHabets
Copy link
Owner

Nothing per-key is stored in the TPM, that's right. The TPM has the possibility to do this since it has limited flash storage, but for various reasons that's not what I chose.

The key is created on the TPM, by default, but can optionally be generated in software instead, and be imported. See https://github.com/ThomasHabets/simple-tpm-pk11/blob/master/doc/stpm-keygen.1.yodl#L24

tl;dr: TPM spec 1.2 is broken, in that imported keys can be exported (if you have the owner PIN, which you can choose to set to random noise and forget), so I'd probably recommend not using -S.

Yes, primary use case is SSH. Do you have other use cases that would require more things?

It's been a while now since I looked at this, but opencryptoki I think is used in the alternative, and opencryptoki is essentially broken beyond repair.

@danni
Copy link

danni commented Jun 9, 2017

Yep, that solution makes sense to me, re not using the NVRAM. I agree as well that the implementation of Opencryptoki appears to defeat the purpose of using the TPM.

I don't have a specific use case, beyond there's a bunch of stuff I was hoping to see exposed via PKCS#11, key generation for instance. But I appreciate that it can just be beyond the scope of what this module aims to do. I did have a quick look at Google Chaps, but it doesn't seem straight forward to get working.

@ThomasHabets
Copy link
Owner

I'd rather not add functionality just for the sake of checking all the boxes, but other specific use cases could definitely be in scope.

I looked at Chaps, but at the time it was not ready. And like you seem to have noticed as well, it has a lot of moving parts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants