-
Notifications
You must be signed in to change notification settings - Fork 1
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
Adding example C client application for Veraison challenge response exchange. #2
base: main
Are you sure you want to change the base?
Conversation
…xchange This application uses libcurl for making REST api calls to the veraison services. Fixes #1 Signed-off-by: Tushar Khandelwal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Tushar! This is a great start.
Could we move this to an example/
folder?
We could then (in another PR) work on factoring out a C interface that could be linked from other C/C++ code.
Signed-off-by: Tushar Khandelwal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments inlined
examples/client.c
Outdated
} | ||
} | ||
|
||
sprintf(uri,"http://%s:%s/challenge-response/v1/",address,port); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to use snprintf
here otherwise, depending on how big address
and port
are, uri
could overflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/client.c
Outdated
|
||
char *new_session="newSession?nonceSize=32"; | ||
char url[100]; | ||
sprintf(url,"%s%s",uri,new_session); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto here for snprintf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/client.c
Outdated
|
||
/* Initialize the curl library */ | ||
curl_handle = curl_easy_init(); | ||
if(curl_handle) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the elders say: "flat is better than nested" :-)
Do an early return here:
if (!curl_handle) {
// error path
}
// happy path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/client.c
Outdated
if(res != CURLE_OK) { | ||
printf("\nerror: %s\n", curl_easy_strerror(res)); | ||
return res; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this else
is not really needed and can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/client.c
Outdated
if(fp!=NULL) { | ||
num = fread(token, 1 ,sizeof(token),fp); | ||
} else { | ||
printf("\ntoken file not found\n"); | ||
return -1; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unneeded nesting, you can do:
if (fp == NULL) {
return -1
}
num = fread(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/client.c
Outdated
headers = curl_slist_append(headers, "Content-Type: application/psa-attestation-token"); | ||
curl_slist_append(headers, "Host: veraison.example"); | ||
curl_slist_append(headers, "Accept: application/vnd.veraison.challenge-response-session+json"); | ||
curl_easy_setopt(curl_handle, CURLOPT_HTTPHEADER, headers); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure whether it's required (or the subsequent perform
will tell us that in any case) but I'd have thought that the return code from curl_easy_setopt
should be checked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
examples/client.c
Outdated
curl_easy_setopt(curl_handle, CURLOPT_HTTPHEADER, headers); | ||
FILE *fp=fopen(filename,"rb"); | ||
int num; | ||
if(fp!=NULL) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the dual fclose
seems to be missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: Tushar Khandelwal <[email protected]>
Signed-off-by: Tushar Khandelwal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments.
I have a number of notes on resource handling but, at this stage, they are not blocking because once the example code exits the OS will take care of it.
We'll tackle that in a subsequent PR, when we expose the interface.
This application uses libcurl for making REST api calls to the veraison services.
Fixes #1
Signed-off-by: Tushar Khandelwal [email protected]