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.

Sign up for GitHub

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

Jump to bottom

Fix krb5 library SO name in the gss api shim #59526

Merged
merged 1 commit into from
Sep 24, 2021
Merged

Fix krb5 library SO name in the gss api shim #59526

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libraries/Native/Unix/System.Net.Security.Native/pal_gssapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ static void* volatile s_gssLib = NULL;
#define GSS_C_NT_HOSTBASED_SERVICE (*GSS_C_NT_HOSTBASED_SERVICE_ptr)
#define gss_mech_krb5 (*gss_mech_krb5_ptr)

#define gss_lib_name "libgssapi_krb5.so"
#define gss_lib_name "libgssapi_krb5.so.2"
Copy link
Member

Choose a reason for hiding this comment

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

Can we align it with how it's done for numa/ssl?

try dlopen libgssapi_krb5.so.2
then try dlopen libgssapi_krb5.so.2.2
then try dlopen libgssapi_krb5.so

Copy link
Member Author

Choose a reason for hiding this comment

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

For SSL, it is different as there are several incompatible versions of the library we support. We always use a versioned one though. But adding fallback to libgssapi_krb5.so sounds probably fine, even though I am not aware of a distro where it would be named that way and usually a change in the major version of the library means a compatibility break. So loading just the .so if present might result in hard to debug crashes in case e.g. libgssapi_krb5.so.3 was released and had some subtle incompatible changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

@am11, it actually seems that for the NUMA case, attempting to use the versionless .so is also something that we might not want to do for the same reason I've mentioned. And do you remember why you have also added the probe for libnuma.so.1.0.0? Have we found systems where the libnuma.so.1 was not present? The comment on the PR that added the libnuma probing is actually not right, there is also libnuma.so on Fedora, the package is numactl-devel.

Copy link
Member

Choose a reason for hiding this comment

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

@janvorli, good points, I think krb5's .2 and numa's .1 are undisputed, other two variants (version-less and .krb5 2.2 / numa .1.0.0) are probably not too great to probe.

The reason of probe for actual binary (1.0.0) was that at the time I was making a .NET example for nanos unikernel docs https://github.com/nanovms/ops-examples/tree/master/dotnet and trying to reduce extra files in that environment (which strives for minimalism). Now I am realizing we can update nanos' packaging tool (ops) so it follows symlink and add both in the resultant image (which we can boot on azure/aws/google hypervisor).


static int32_t ensure_gss_shim_initialized()
{
Expand Down