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

kbs/plugins/resource: Add ID_KEY resource backend #698

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tylerfanelli
Copy link
Contributor

@tylerfanelli tylerfanelli commented Feb 6, 2025

The ID_KEY resource backend creates a new key tied to a Trustee server instance and mostly pertains to workloads that want a key unwrapping as a result of a successful attestation.

Admins can POST to the repository giving a base64 encoded byte vector as the resource tag. This resource tag will then be encrypted with the key and returned to the admin. Upon successful attestation, attestation clients can then send the encrypted data that was provisioned by the admin to the backend via a GET request. The resource tag in this case must be a base64 encoding of the encrypted data's bytes. The data will then be decrypted and sent back to the attestation client.

I will likely provide another PR in the near future allowing the server's encryption key to be derived from an HSM module for security purposes. While it won't be required, it will be preferred.

@tylerfanelli tylerfanelli requested a review from a team as a code owner February 6, 2025 02:49
@fitzthum
Copy link
Member

fitzthum commented Feb 7, 2025

I'll take a look at the code tomorrow.

On a high level, what does this give you that you can't get from an existing backend? Let's say I'm an admin. I post some bytes to the plugin and get back the wrapped bytes. I give those wrapped bytes to my guest. Then the guest can get the unwrapped version from Trustee. How is that different from an admin simply uploading a secret to Trustee and then giving the client the name of the secret?

Also, are you worried about replay attacks?

@tylerfanelli
Copy link
Contributor Author

tylerfanelli commented Feb 7, 2025

I'll take a look at the code tomorrow.

On a high level, what does this give you that you can't get from an existing backend? Let's say I'm an admin. I post some bytes to the plugin and get back the wrapped bytes. I give those wrapped bytes to my guest. Then the guest can get the unwrapped version from Trustee. How is that different from an admin simply uploading a secret to Trustee and then giving the client the name of the secret?

The issue with this is that there's no way of preventing the client from fetching a secret that they're not supposed to. If there is more than one VM attempting to attest, there's no stopping them from requesting the secrets of another VM. This simplifies it by only allowing that VM's secret available to it. We only care about one secret in this scenario.

In other words, if an attacker knows the secret name, it can't just try to request it itself. The standard backend is a bit susceptible to this. This offers isolation, as it doesn't even really store any secrets, but relies on the client to store them.

It also allows a key to be derived from an HSM (not a part of this series). If the admin also has access to the HSM, they can verify their "root key" is never compromised yet was used to encrypt data (in our case, block storage).

Also, are you worried about replay attacks?

Yep. A man-in-the-middle could sniff the URL and just replay to get the decrypted secret. This is functional but unsafe, I'm thinking of adding a ECDH (ephemeral key) exchange. The public keys could be supplied via the request's body.

@tylerfanelli tylerfanelli force-pushed the resource-idkey branch 4 times, most recently from a10c871 to cadd86d Compare February 7, 2025 07:00
@tylerfanelli tylerfanelli marked this pull request as draft February 7, 2025 07:01
Other plugins may want to return a value once a resource has been added
to the backend.

As no backend uses this feature yet, they all return a None value for
return values. This means there's no functional changes to any other
backend due to this modification.

Signed-off-by: Tyler Fanelli <[email protected]>
Implement the read_resource methods on the backend.

Signed-off-by: Tyler Fanelli <[email protected]>
To prevent replay attacks, the ID_KEY backend will receive encrypted
keys wrapped with ECDH ephemeral keys. To get the client's EC key, the
client will supply it via the request body. Allow the request body to be
passed to the resource read handler.

No other resource backends use the request body at present, so the
argument is ignored and the behavior is unchanged.

Signed-off-by: Tyler Fanelli <[email protected]>
To prevent replay attacks, the ID-KEY resource backend requires an ECDH
public key exchange. Create a ECDH secret key and allow others to fetch
the public key through a GET request. Read ECDH data from clients from
the request body and use data to complete exchange.

Signed-off-by: Tyler Fanelli <[email protected]>
@tylerfanelli
Copy link
Contributor Author

This is now ready for review, but has a few blockers before it can be considered for merging. You will notice the final commit is prefixed by a FIXME tag. To facilitate the ECDH exchange, I've added a {de}serializable type to kbs-types for the data needed. The PR is here: virtee/kbs-types#57

I've included the type (as well as the associated {de}serialization methods usually found in kbs-types) in the id-key module, but once the kbs-types PR is merged, it can be removed. However, the PR is also based on a later version of kbs-types that broke the API, so I'm also blocked by #597 (which updates the API and kbs-types version) as well. Once both of these are handled, this will be ready for merging.

@fitzthum When you have the chance, would you mind reviewing?

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 this pull request may close these issues.

2 participants