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

Add list-profiles sub-command to p11-kit cmd tool #500

Merged
merged 2 commits into from
May 3, 2023

Conversation

ZoltanFridrich
Copy link
Contributor

@ZoltanFridrich ZoltanFridrich commented Apr 26, 2023

Adds list-profiles command for listing profiles that are supported by a token
usage: p11-kit list-profiles pkcs11:token

@coveralls
Copy link

coveralls commented Apr 26, 2023

Coverage Status

Coverage: 70.448% (+0.07%) from 70.38% when pulling 877dadc on ZoltanFridrich:zfridric_devel4 into 3f2c94f on p11-glue:master.

p11-kit/list-profiles.c Outdated Show resolved Hide resolved
p11-kit/meson.build Show resolved Hide resolved
p11-kit/list-profiles.c Outdated Show resolved Hide resolved
p11-kit/list-profiles.c Outdated Show resolved Hide resolved
common/constants.c Outdated Show resolved Hide resolved
@ZoltanFridrich ZoltanFridrich force-pushed the zfridric_devel4 branch 3 times, most recently from 9d222e8 to 4c411fa Compare April 26, 2023 15:02
@ZoltanFridrich ZoltanFridrich requested a review from ueno April 26, 2023 15:13
Copy link
Member

@ueno ueno left a comment

Choose a reason for hiding this comment

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

I think this PR really deserves a test case. The below is what I used for testing it locally, but ideally there should be a test integrated into the test suite:

  1. Modify p11-kit/mock-module-ep.c
diff --git a/p11-kit/mock-module-ep.c b/p11-kit/mock-module-ep.c
index 4324433..4845dcd 100644
--- a/p11-kit/mock-module-ep.c
+++ b/p11-kit/mock-module-ep.c
@@ -39,6 +39,14 @@
 
 #include "mock.h"
 
+static CK_RV
+override_initialize (CK_VOID_PTR init_args)
+{
+       CK_RV rv = mock_C_Initialize (init_args);
+       mock_module_add_profile (MOCK_SLOT_ONE_ID, CKP_PUBLIC_CERTIFICATES_TOKEN);
+       return rv;
+}
+
 #ifdef OS_WIN32
 __declspec(dllexport)
 #endif
@@ -47,6 +55,7 @@ C_GetFunctionList (CK_FUNCTION_LIST_PTR_PTR list)
 {
        mock_module_init ();
        mock_module.C_GetFunctionList = C_GetFunctionList;
+       mock_module.C_Initialize = override_initialize;
        if (list == NULL)
                return CKR_ARGUMENTS_BAD;
        *list = &mock_module;
  1. Build
meson setup _build -Dprefix=/usr
meson compile -C _build
  1. Copy mock-one.so and one.module
cp _build/p11-kit/mock-one.so /usr/lib64/pkcs11
cp p11-kit/fixtures/system-modules/one.module /usr/share/p11-kit/modules
  1. Run p11-kit list-profiles
_build/p11-kit/p11-kit list-profiles 'pkcs11:model=TEST%20MODEL;manufacturer=TEST%20MANUFACTURER;serial=TEST%20SERIAL;token=TEST%20LABEL'

common/constants.c Show resolved Hide resolved
@ZoltanFridrich ZoltanFridrich changed the title Add command to list token profiles Add list-profiles sub-command to p11-kit cmd tool Apr 27, 2023
@ueno
Copy link
Member

ueno commented Apr 28, 2023

One way to test this would be to create a separate p11-kit executable for testing (e.g., p11-kit-testable), which links to libp11-kit-testable.a. The it should be able to access the mock modules installed under p11-kit/fixtures/.

@ZoltanFridrich
Copy link
Contributor Author

One way to test this would be to create a separate p11-kit executable for testing (e.g., p11-kit-testable), which links to libp11-kit-testable.a. The it should be able to access the mock modules installed under p11-kit/fixtures/.

I thought that I might do it similar to test-iter where I would include the list-profiles.c and supply my own argv argc to the p11_kit_list_profiles, then I would redirect the stdout through pipe and read whether it contains correct profiles.

@ueno
Copy link
Member

ueno commented Apr 28, 2023

I thought that I might do it similar to test-iter where I would include the list-profiles.c and supply my own argv argc to the p11_kit_list_profiles, then I would redirect the stdout through pipe and read whether it contains correct profiles.

I'm not sure if it makes sense to test commands in C rather than simple shell scripts, given we intend to add more subcommands (e.g., #394). With shell script, you can simply diff the output against the expected one.

@ZoltanFridrich ZoltanFridrich force-pushed the zfridric_devel4 branch 4 times, most recently from c33bfb9 to f22d207 Compare May 2, 2023 13:29
@ZoltanFridrich ZoltanFridrich requested a review from ueno May 2, 2023 14:14
p11-kit/meson.build Outdated Show resolved Hide resolved
@ZoltanFridrich ZoltanFridrich force-pushed the zfridric_devel4 branch 3 times, most recently from 7b4dcde to 1a77393 Compare May 3, 2023 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants