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

AA: get_token API add runtime_data paramter #465

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jialez0
Copy link
Member

@jialez0 jialez0 commented Feb 1, 2024

Let structured runtime data can be passed to AA when call it to get CoCo-AS token. This is useful for requesting KBS resources with CoCo AS token, because we need this parameter to bind TEE public key with TEE evidence and let TEE public key is included in CoCo-AS token.

cc @fitzthum @mkulke

api-server-rest/openapi/api.json Outdated Show resolved Hide resolved
attestation-agent/lib/src/token/coco_as.rs Outdated Show resolved Hide resolved
@jialez0 jialez0 force-pushed the fix-get-token branch 2 times, most recently from 25d8bae to 4817259 Compare February 2, 2024 07:22
Copy link
Member

@portersrc portersrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me, but I lack a lot of context. One question so I can learn: When coco_as.rs's get_token invokes client.post, where does that post hit? Is it roughly in here? (Basically I'm trying to see where the receiver handles this new structured runtime data.)

attestation-agent/lib/src/token/coco_as.rs Outdated Show resolved Hide resolved
Copy link
Member

@fitzthum fitzthum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think this is a better approach than confidential-containers/trustee#307. Honestly it took me a couple days to understand what was going on here. Sorry for the delay.

Now that I have clearer picture, I am starting to wonder if it's actually worth making changes to the passport model. I am remembering some of the reasons why we implemented it with two KBSes in the first place and I'm skeptical about changing to just one. In general what do you see as the costs of the first approach? I think having two KBSes might be a little counterintuitive, but beyond that I'm not sure what the issues are.

let structured_value: serde_json::Value = serde_json::from_str(structured_runtime_data)
.context("Get Token Failed: Structured Runtime Data must be a JSON Map")?;

// TODO: Request AS to get Nonce and insert the Nonce into structured runtime data JSON Map.
Copy link
Member

@fitzthum fitzthum Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even once the AS provides the nonce for the runtime data, how is the KBS supposed to verify the freshness? Normally in RCAR the KBS provides the nonce as part of the challenge, then the attestation must include the nonce. The KBS then insures that the nonce in the attestation matches the nonce that it chose for the challenge. The flow you have here isn't RCAR, it's just A, which I don't think is going to work. Even once you add an additional call to the AA to get a nonce, you still wouldn't be able to verify that the nonce was actually used.

I think we have a somewhat fundamental issue here. I think to be secure the protocol for connecting to the AS would need to look a lot more like RCAR. As in, the AA would return a token similarly to how a KBS would return a resource. I would envision

  1. request a token from the AS
  2. AS gives back a nonce
  3. AA sends the evidence
  4. AS checks the nonce (and the evidence) and returns a Token

One issue is that currently the AS is not stateful so I don't know how it would do step 4. Looking at the steps, this is basically exactly what the KBS does. So I wonder if we should really bother with replacing it.

cc: @Xynnn007

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. A challenge-response protocol like RCAR (which requires 4-round communication) would effciently prevent replay-attack in CoCoAS and I think @jialez0 will add that support inside CoCoAS later.

Another question is how we design the API of get_token().
Try to think about this: if we already had a challenge-response protocol in CoCoAS, the challenge-response protocol could be handled inside the CoCoAS' get_token() handler. The only reason why we need the API of get_token() to have extra runtime_data argument is to specify the TEE public key. This is somehow a complicated situation.

Overall, KBS and CoCoAS should be different separate component. We can embed the public key claims as runtime data to request CoCoAS to check integrity, but

  1. this requires the caller/user of AA to prepare the key pair manually
  2. the actual runtime data would be two parts. One is from the parameter runtime_data like the public key and the other is the nonce got from CoCoAS during the protocol logic inside get_token() function.

The reason why we face the delima is that we try to couple AS token's with KBS token.

AS token semantically means the endorsement of the evidence, and the claims against the reference values/policies are correct.

KBS token semantically means it is authorized to access specific resources. It is hard to say AS token have ability to work as an authorized token.

The coincidence is that they are the same now. TBO we now are using them without distinguishment, but it works fine so far regardless of the coupling.

My personal preference, in short term is

  1. Still have the extra runtime_data parameter to get_token().
  2. leave current coco-as token logic as what is implemented in this PR. This means this handler would support user-defined random runtime claims. The usage is defined by the caller.
  3. Adds another token type whose get_token() will support public key generation, handshake with CoCoAS, and return the token with format that KBS recognizes. The handler of this token will not care about the given runtime_data parameter. This seems like a workaround before we have a more clear boundary between KBS token and AS token.

In long term, we should carefully think about the authentication and authorization of attestation system, including

  1. The authentication and authorization flow of KBS resources based on attestation, where the independence of individual components can be maintained without coupling
  2. The format and usages of different token. Whether they can be used interchangeably? If so, how? If not, what is the boundary?

We'd better design this align with current standards without inventing new wheels. After initdata, we will have richer toolbox to achieve this.

does this make sense? @fitzthum @jialez0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right that we are conflating the KBS Token and the AS Token. I think we're also trying to split the RCAR protocol between three different parties. This is very confusing.

Since we already have a way to support a passport mode, I am hesitant to go forward with something that isn't clear. I know that the existing approach is not semantically correct, but otherwise it seems like it works.

Here is one way we could think about passport mode with only one KBS. Maybe we should think about the AS token not as something that can stand in for the KBS token, but as something that can stand in for evidence. There is one problem with this approach but generally I think it makes sense.

Basically, the AA would still go through a full RCAR flow with the KBS, but instead of providing hw evidence in the attestation, it would provide the AS token. Then instead of contacting the AS to verify the attestation, the KBS would use this AS token directly.

The issue here is that if the AS token were generated before connecting to the KBS, I'm not sure how we would bind it to the public key and nonce needed in the RCAR.

Anyway, enjoy the new year. Afterwards maybe we can try to draw some diagrams.

@jialez0
Copy link
Member Author

jialez0 commented Feb 19, 2024

does this make sense? @fitzthum @jialez0

@Xynnn007 Okay, I agree with you. I will implement the new token type in the subsequent PR and configure the token type through env as you mentioned in PR #466.

Can we merge this PR first?

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.

5 participants