smallstep / step-kms-plugin

🔐 step plugin to manage keys and certificates on a cloud KMSs and HSMs
Apache License 2.0
49 stars 6 forks source link

Add support for `TSS2 PRIVATE KEY` PEM format #73

Closed hslatman closed 10 months ago

hslatman commented 1 year ago

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 https://github.com/smallstep/step-kms-plugin/pull/71.

dwmw2 commented 1 year ago

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 commented 1 year ago

cf. https://gitlab.com/gnutls/gnutls/-/issues/594#note_688892934

dwmw2 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 11 months ago

I've started the support of TSS2 files at https://github.com/smallstep/crypto/pull/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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

@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 https://github.com/smallstep/crypto/pull/357