KBS /resource: support CoCo-AS token without public key in it #307
Conversation
| .body(res)) | ||
| // If public key is included in claims, use JWE to wrap resource byte, | ||
| // else check if HTTPS is enabled. | ||
| match get_pkey(claims) { |
There was a problem hiding this comment.
If I understand correctly we want to support requests without public key for KBS deployments without HTTPs. In this case there would never be a be pkey, right? Does it still make sense to attempt to fetch it or should we rather start the with the HttpsEnabled == false check and return the unwrapped resource? (Instead of swallowing the get_pkey() error).
There was a problem hiding this comment.
As you mentioned, this PR hopes to support requests without pubkey in KBS with HTTPS enabled. However, "HTTPS enabled" is a minimum security measure. Even if HTTPS is enabled, pubkeys may still exist. In this case, the returned resource data is protected by both HTTPS and JWE. Only when the pubkey cannot be found, data will be returned when HTTPS is enabled. If HTTPS is not enabled, it is completely unsafe, and KBS will refuse to return data.
There was a problem hiding this comment.
I see. thanks for clarifying. So, it looks like having no pubkey in the claims is a legitimate code path, i.e. the signature is actually something like: fn get_pkey() => Option<Result<TeePubKey>>. A result should capture unexpected states. An alternative would be to match on a concrete error type:
Err(err) => {
if let err = Error:NoPubkeyInClaims && https.enabled {
....
}
Err(Error::InsecureResponse)
}Failing to parse ( TeePubKey::deserialize(pkey_value)) should probably result in an error and not in a fallback to https, because this shouldn't happen in any case.
If customized_claims is specified as always having runtime_data.tee-pubkey we probably want to error out, too.
Signed-off-by: Jiale Zhang <[email protected]>
| .body(res)) | ||
| // If public key is included in claims, use JWE to wrap resource byte, | ||
| // else check if HTTPS is enabled. | ||
| match get_pkey(claims) { |
There was a problem hiding this comment.
I see. thanks for clarifying. So, it looks like having no pubkey in the claims is a legitimate code path, i.e. the signature is actually something like: fn get_pkey() => Option<Result<TeePubKey>>. A result should capture unexpected states. An alternative would be to match on a concrete error type:
Err(err) => {
if let err = Error:NoPubkeyInClaims && https.enabled {
....
}
Err(Error::InsecureResponse)
}Failing to parse ( TeePubKey::deserialize(pkey_value)) should probably result in an error and not in a fallback to https, because this shouldn't happen in any case.
If customized_claims is specified as always having runtime_data.tee-pubkey we probably want to error out, too.
|
|
||
| pub(crate) fn get_pkey(claims: Value) -> Result<TeePubKey> { | ||
| let pkey_value = claims | ||
| .get("customized_claims") |
There was a problem hiding this comment.
Can we define/reuse a type CustomizedClaim instead of manually performing the parsing by stepping through properties?
| return Err(Error::InsecureResponse); | ||
| } | ||
| let res = URL_SAFE_NO_PAD.encode(resource_byte); | ||
| Ok(HttpResponse::Ok().body(res)) |
There was a problem hiding this comment.
I am confused by this PR. Isn't it fundamental that the secure channel is bound to the hw evidence? In other words, doesn't the KBS want to know that when it encrypts a value using a public key, it can only be decrypted inside the enclave that has the corresponding private key? I'm not sure it's valid to support secret delivery without this. What use case are you looking at?
There was a problem hiding this comment.
@fitzthum I am trying to get CDH to support calling AA to obtain a token from AS, and then using this token to access KBS to obtain resources. However, currently, AA does not carry a pubkey in the tokens obtained from AS.
Another possible path is to add a runtime data parameter to the API for obtaining AS tokens for AA to pass the pubkey, and CDH is responsible for generating and managing this pubkey, so that KBS can still wrap and return resources using JWE.
3 participants
No description provided.