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

Add support for TSS2 PRIVATE KEY PEM format #73

Closed
hslatman opened this issue Jun 13, 2023 · 11 comments · Fixed by #107
Closed

Add support for TSS2 PRIVATE KEY PEM format #73

hslatman opened this issue Jun 13, 2023 · 11 comments · Fixed by #107
Assignees

Comments

@hslatman
Copy link
Member

hslatman commented Jun 13, 2023

To further improve interoperability with other TPM2 tooling, such as tpm2_tools, we should add support for exporting TPM keys in the PEM TSS2 PRIVATE KEY format. Importing that same format would be nice too. A document describing the format is available here.

Exporting can probably be done by printing the key in the right PEM format to stdout, so that the contents can be written to a file by redirecting the output. Importing would require reading from file or stdin, parsing the PEM, then converting it to our (external) key format, so that it can be serialized into our own format.

We probably want to support the core of these operations in go.step.sm/crypto, so that it can be reused. Parts of it should go in pemutil. Others in the kms/tpmkms and tpm packages.

In step-kms-plugin we would need to add the format as an option for export, and ensure that the PEM file can be read for import.

An extension of this would be to support TSS2 PRIVATE KEY PEM as a "native" storage format in the tpm/storage package. We'll likely need to store some additional (meta)data for a private key outside of the base64 in the PEM file. Headers and/or before/after the ----BEGIN and -----END anchors could work for that. Separate file(s) could also work.

Some more context is available in the discussion for #71.

@hslatman hslatman self-assigned this Jun 13, 2023
@dwmw2
Copy link

dwmw2 commented Jun 13, 2023

Note that the transient parent you generate for the PEM keys in their default mode needs to precisely match the one from the TPMv2 specifications. Life is easier if you use the right parameters by default for all your keys (tpm-pkcs11 didn't, which made it harder to export their keys to PEM format. I think they do now).

If you don't have time to look at anything else in the short term, getting that right for the future would be good.

@dwmw2
Copy link

dwmw2 commented Jun 13, 2023

@dwmw2
Copy link

dwmw2 commented Jun 13, 2023

If it's useful to crib from https://gitlab.com/openconnect/openconnect/-/blob/master/gnutls_tpm2.c then I'm happy to relicense it. It should be sufficiently documented that you can use it to write a Go version. I may even try to help, but you might not enjoy my attempts at Go.

@hslatman
Copy link
Member Author

hslatman commented Jun 13, 2023

Note that the transient parent you generate for the PEM keys in their default mode needs to precisely match the one from the TPMv2 specifications. Life is easier if you use the right parameters by default for all your keys (tpm-pkcs11 didn't, which made it harder to export their keys to PEM format. I think they do now).

If you don't have time to look at anything else in the short term, getting that right for the future would be good.

Our tpmkms implementation relies on our tpm package, which in turn relies on go-attestation (primarily; for attestation use cases) and go-tpm. For go-attestation, keys (attestation as well as application) are created under the SRK handle (0x81000001). There's templating code for the SRK, which I think does the right thing. Do you think that could be an issue? At the moment go-attestation doesn't allow to specify a custom hierarchy of keys, so that would entail rewriting parts of our implementation on top of go-tpm, instead.

There's work close to being completed in go-tpm that makes the interface with the TPM more pleasant to work with that I think could benefit that type of thing too, so we would probably await that at least.

@jejb
Copy link

jejb commented Jun 13, 2023

keys (attestation as well as application) are created under the SRK handle (0x81000001)

If you work with a wide variety of TPM systems, you'll find this to be a problem because the SRK is often unprovisioned. Various attempts to correct this in distro packaging (run a create primary and then store in the missing index) have been rebuffed because an RSA createPrimary can take minutes and users are too impatient. The upshot is the ephemeral parent scheme where you run a createPrimary on the P-256 curve to a standard TCG mandated template and use that key as the parent, which gets you out of having to have any persistent primaries in the NV ram.

The TPM key spec supports both persistent and ephemeral parents, so it will definitely work for you, but you should be aware of the problem because it likely means that for interoperability you'll have to handle private keys with ephemeral parents.

@dwmw2
Copy link

dwmw2 commented Jun 15, 2023

FWIW I started throwing some of this together in https://github.com/dwmw2/rolesanywhere-credential-helper/blob/tpm/aws_signing_helper/tpm_signer.go

It's Apache v2 licensed so feel free to use whatever you like of it. Ideally I suspect it should live in go-tpm though; we shouldn't be having to do any of this for ourselves.

@maraino
Copy link
Contributor

maraino commented Oct 27, 2023

I've started the support of TSS2 files at smallstep/crypto#353. One of the initial problems that I encountered is that OpenSSL seems to encode a boolean TRUE as 0x01 instead of 0xff, probably due to confusion between DER and BER formats, BER is more relaxed and allows any other value besides 0x00 to be TRUE.

@dwmw2
Copy link

dwmw2 commented Oct 28, 2023

Yeah, we've fixed that in both engines and I have a (fairly nasty) workaround in https://github.com/dwmw2/rolesanywhere-credential-helper/blob/tpm/aws_signing_helper/tpm_signer.go#L242

@maraino
Copy link
Contributor

maraino commented Oct 28, 2023

Yeah, we've fixed that in both engines and I have a (fairly nasty) workaround in https://github.com/dwmw2/rolesanywhere-credential-helper/blob/tpm/aws_signing_helper/tpm_signer.go#L242

In my PR, instead of using asn1.Unmarshal(), I'm using the x/crypto/cryptobyte package. Go standard library is now using that in the new parsers, for example, to parse an x509 certificate:
https://github.com/golang/go/blob/8c92897e15d15fbc664cd5a05132ce800cf4017f/src/crypto/x509/parser.go#L800-L813

Note that some parts of my parsing method are totally untested, mainly the optional policies and auth policies. Examples of keys or steps to create those will be appreciated.

For the marshaling, I'm just currently using asn1.Marshall() instead of a cryptobyte.Builder the keys will have the emptyAuth set to 0xFF, at the moment, I've been able to use OpenSSL with those keys without issues.

In any case there are still many things pending in that PR, but feel free to use my parsing instead of patching the PEM.

@maraino
Copy link
Contributor

maraino commented Nov 1, 2023

Hi @dwmw2, I've tried to test your signer integration without success. It always fails when it tries to load a key using tpm2.Load().

With your code, with *key[2:], I get the error parameter 1, error code 0x1f : integrity check failed. If I pass the full key, I get parameter 2, error code 0xa : the type of the value is not appropriate for the use. It does not matter if I pass just the full public or both at the same time, I always get the same error.

Have you encountered the same issue?

I've tried creating a key using tpm2.CreateKey, replacing the bytes, and their size was 2 bytes smaller, but they worked perfectly. So I'm guessing it can be related to the key handler, but I've been playing with the primaryParams without luck.

Another thing that I want to mention is that a proper implementation of a crypto.Signer receives the digest from the input, you don't have to calculate it inside. It should be something like this:

hash := crypto.SHA256.New()
hash.Write([]byte("the message foo"))
sum := hash.Sum(nil)

sig, err := signer.Sign(rand.Reader, sum[:], crypto.SHA256)

But there is one exception, if you sign with an Ed25519 key, you should pass the full message. But for TPMs you should be passing the digest.

@maraino
Copy link
Contributor

maraino commented Nov 2, 2023

@dwmw2, it was a problem with how I was creating the keys using tpm2-tools. I still need to collect all the steps necessary, but for now, if I create them using my code, I can use the signer without issues. The new PR with that is in smallstep/crypto#357

@maraino maraino mentioned this issue Nov 6, 2023
1 task
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 a pull request may close this issue.

4 participants