-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add more pkcs11 tests #426
Conversation
@@ -665,7 +642,7 @@ int aws_pkcs11_lib_find_private_key( | |||
bool must_finalize_search = false; | |||
|
|||
/* set up search attributes */ | |||
CK_OBJECT_CLASS key_class = CKO_PRIVATE_KEY; | |||
CK_OBJECT_CLASS key_class = CKO_SECRET_KEY; |
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.
What's the difference between CKO_PRIVATE_KEY and CKO_SECRET_KEY?
is a PRIVATE_KEY also qualify as a SECRET_KEY?
The tests were passing when I created the private key via:
softhsm2 --import tests/resources/unittests.p8
and searched for a CKO_PRIVATE_KEY
does CKO_SECRET_KEY also work when searching for keys created via the --import
command?
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 discussed offline, its basically the symmetric key which does not have a public key/cert associated with it. I will revert back to asymmetric key.
if (token_dir[s_pkcs11_tester.token_dir->len - 1] == '/') { | ||
sprintf(cmd, "rm -rf %s*", token_dir); | ||
} else { | ||
sprintf(cmd, "rm -rf %s/*", token_dir); |
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.
this isn't cross-platform, but believe it or not we have a code-review in the works right now that adds cross-platform directory obliteration:
awslabs/aws-c-common#830
anyway, that review isn't merged. This is fine for now. we'll use the cross-platform stuff once it's available
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
sprintf(cmd, "rm -rf %s/*", token_dir); | ||
} | ||
printf("Executing command: %s", cmd); | ||
system(cmd); |
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.
check that system() returns 0 ASSERT_SUCCESS(system(cmd))
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.
or use our cross-platform stuff, but it's a lot of boilerplate
https://github.com/awslabs/aws-c-common/blob/main/include/aws/common/process.h
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.
nice
aws_pkcs11_lib_release(pkcs11_lib); | ||
s_pkcs11_tester_clean_up(); | ||
return AWS_OP_SUCCESS; | ||
} | ||
AWS_TEST_CASE(pkcs11_find_private_key, s_test_pkcs11_find_private_key) | ||
|
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.
if you can break tests up at all, please do so
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.
Will do that
This is necessary for incoming tests: #426 It's also nice to use the typedefs privately, instead of uint64_t for everything.
Issue #, if available:
Description of changes:
** Find slot tests
** Find key tests (TODO: Add more key types/lengths)
** Login tests
** Session tests
TEST_OUTPUT
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.