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 in the ability to dynamically adjust module and configuration paths at run-time via system environmental exports #442

Merged
merged 2 commits into from
Jun 1, 2023

Conversation

timesys-nathan
Copy link
Contributor

Add in the ability to dynamically adjust module and configuration paths at run-time via system environmental exports

Background: While trying to integrate PKCS11+HSM into a build system (Yocto Project), we wanted the build system to be able to use the HSM directly. However, The Yocto Project creates a sandboxed environment for each package which uses p11-kit during compilation. This means that the p11-kit modules and configurations cannot be found from within these sandboxed directories, as their paths are non-determinable at compile time. OpenSSL appears to use OPENSSL_CONF for this sort of dynamic configuration, so I've followed their lead and incorporated a similar p11_kit_getenv() method.

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.

There is p11_kit_override_system_files for this purpose and I remember it was a deliberate choice that we don't support setting them through envvars. Would it be possible to workaround this by either using the function or bind-mounting one of the module directories?

*/

static char *
p11_kit_getenv(const char *name)
Copy link
Member

@ueno ueno Nov 8, 2022

Choose a reason for hiding this comment

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

We can assume that secure_getenv is always defined, as common/compat.c provides the same fallback if configure/meson couldn't find it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks -- I've removed the unnecessary check

…hs at run-time via system environmental exports

Background: While trying to integrate PKCS11+HSM into a build system (Yocto Project), we wanted the build system to be able to use the HSM directly.  However, The Yocto Project creates a sandboxed environment for each package which uses p11-kit during compilation.  This means that the p11-kit modules and configurations cannot be found from within these sandboxed directories, as their paths are non-determinable at compile time.  OpenSSL appears to use OPENSSL_CONF for this sort of dynamic configuration, so I've followed their lead and incorporated a similar p11_kit_getenv() method.
@timesys-nathan
Copy link
Contributor Author

@ueno

Bind mounting or chrooting would work in theory, but it's not very compatible with how Yocto works. There's not a good way to bind mount/chroot the sandboxed build directories during the process.

I did see p11_kit_override_system_files(), but that would also require patching all of the subsequent components which use libp11-kit.so separately. It would seem to me that the simpler solution would be to be able to change the paths inside one library, instead of adjusting it in multiple places.

I've updated my merge request and added in a config option called env_override_paths, which should be disabled by default. Would this be acceptable? I fully understand that, by default, you may not want environmental overrides to enabled. We're certainly dealing with a minority edge-case here, where 99.9% of people using this package will not need the override support. So, let's make it an option and keep it off by default?

meson _build -Denv_override_paths=enabled && meson compile -C _build

This would work fine for us.

@timesys-nathan timesys-nathan requested a review from ueno November 9, 2022 14:04
Copy link
Contributor

@ZoltanFridrich ZoltanFridrich left a comment

Choose a reason for hiding this comment

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

The changes look good. The formatting is not consistent with p11-kit format but I can fix that after the merge. Approved.

@coveralls
Copy link

Coverage Status

Coverage: 69.971% (-0.002%) from 69.972% when pulling d6849ab on timesys-nathan:master into 8a305a0 on p11-glue:master.

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.

4 participants