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

verifiers: use type instead of name in static init #89

Conversation

JamesMenetrey
Copy link
Contributor

Dear developers,

Following my previous PR (#82), I realized that the code previously mentioned by @KB5201314 was also causing issues for validation of attestation evidence when using librats in WAMR:

err = rats_verifier_select(ctx, choice);

Indeed, the second argument, choice, is previously assigned to the type of a verifier:

librats/core/rats_init.c

Lines 158 to 160 in 42e2d7d

choice = ctx->config.verifier_type;
if (choice[0] == '\0') {
choice = rats_global_core_context.config.verifier_type;

Hence, the function call to rats_verifier_select must be swapped with rats_verifier_select_by_type.


For a comprehensive description of the issue: in WAMR, the verifier of type sgx_ecdsa is used for quotes verification, but only the qve variant is provided. An error is raised during the validation process, as detailed in the logs below.
By requesting the type instead of the name for the verifier, the type sgx_ecdsa is recognized for its variant qve, which is then provided and working as expected.

[ERROR] rats_verifier_select()@L54: failed to select the rats verifier of name 'sgx_ecdsa'
[ERROR] librats_verify_evidence_from_json()@l145: failed to librats_verify_evidence return 0x30000007
ERROR: Evidence is not trusted, error code: 0x30000007.

@imlk0
Copy link
Collaborator

imlk0 commented Aug 27, 2023

Thank you again @JamesMenetrey . We have successfully reproduced this bug in WAMR.

Here is the key point of this problem:

memcpy(conf.verifier_type, evidence->type, sizeof(conf.verifier_type));

Here evidence->type should be the same as the verifier's type field (for sgx evidence it should always be "sgx_ecdsa"). While conf.verifier_type is a user-supplied field (this goes back to the rats-tls days), and should match the verifer's name field (can be "sgx_ecdsa" or "sgx_ecdsa_qve"). So we can't assign evidence->type to conf.verifier_type.

Sorry we can't merge your code. Changing rats_verifier_select() to rats_verifier_select_by_type() in rats_verifier_init() function breaks the other code, where conf.verifier_type is used.

if ((ret = rats_verifier_init(&conf, &ctx)) != RATS_VERIFIER_ERR_NONE)

We've created a new pr #90 to fix this bug and tested it on WAMR.
Thanks for your contribution!

@JamesMenetrey
Copy link
Contributor Author

Hello @KB5201314,

Many thanks for quickly checking the issue and providing another PR to fix the underlying problem!

I tested only my side, and your PR indeed solves the issue, thanks!

Cheers

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