search

Azure / iot-identity-service

Source of the Azure IoT Identity Service and related services.
MIT License
37 stars 46 forks source link

Add CKF_OS_LOCKING_OK flag to C_Initialize args #491

Closed quality-leftovers closed 1 year ago

quality-leftovers commented 1 year ago

Would it be possible to set CKF_OS_LOCKING_OK when calling C_Initialize to enable use of PKCS#11 libraries that cannot do without OS locking? Or is there something in the threading model that disallows use of OS locking?

If you are interested in merging, then l can do some further verification. First time doing changes in rust, so beware :)

Greets

quality-leftovers commented 1 year ago

@microsoft-github-policy-service agree [company="TRUMPF SE"]

quality-leftovers commented 1 year ago

@microsoft-github-policy-service agree company="TRUMPF SE"

arsing commented 1 year ago

Or is there something in the threading model that disallows use of OS locking?

No, there shouldn't be. If anything, the threading model is essentially "PKCS#11 library will be called from only one thread at a time, though not necessarily the same thread every time." so any locking won't matter anyway.

If you are interested in merging, then l can do some further verification. First time doing changes in rust, so beware :)

So yes, I don't see a problem with merging. And yes, changes look fine. I kicked off the CI.

quality-leftovers commented 1 year ago

Ok, thanks. I see that CI is failing due to code formatting.

I'll set up the required tools locally and run the test steps locally and will fix any errors that come up.

quality-leftovers commented 1 year ago

After running rustfmt the package and test step passed in my repo fork.

The e2e action failed for me, because using my branch name the generated name for azure iot hub exceeds the 50 character (Creating IoT Hub: features-pkcs11-init-------------------3621486752-4... -> 51 chars) limit. Not sure if that is a problem and if I should rename my branch.

See test-common.sh:15

test_common_resource_name="$(printf '%s' "$test_id" | tr -C 'a-z0-9' '-')"
arsing commented 1 year ago

The e2e action failed for me, because using my branch name the generated name for azure iot hub exceeds the 50 character

Yes, known issue.

You don't have to rename your branch (which would require opening a new PR). You can just make another branch at the same commit as this one and run the tests on those.

quality-leftovers commented 1 year ago

Ok, understood. Sounds good.

=> E2E worked fine with another branch for same commit )https://github.com/quality-leftovers/iot-identity-service/actions/runs/3630840254/jobs/6137863541 -> identical to https://github.com/Azure/iot-identity-service/actions/runs/3625601230)