Remove stderr logging when libgssapi_krb5 is not available; redirect to NetEventSource.Error#126300
Remove stderr logging when libgssapi_krb5 is not available; redirect to NetEventSource.Error#126300
Conversation
…ging in managed code Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/9153c03c-ec0e-4f65-b140-ed29f159d0f9 Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
| // GSSAPI shim may not be available on some platforms (Linux Bionic) | ||
| return new UnsupportedNegotiateAuthenticationPal(clientOptions); | ||
| } | ||
| catch (TypeInitializationException tie) |
There was a problem hiding this comment.
Where does the TypeInitializationException that this is catching come from? Can this error handling be more specific?
There was a problem hiding this comment.
The TypeInitializationException comes from GssInitializer.Initialize(), which is called by NetSecurityNative's static constructor. Previously GssInitializer's static constructor threw a bare InvalidOperationException which was then wrapped in TypeInitializationException.
Changed in b04fdcb: GssInitializer.Initialize() now throws DllNotFoundException("libgssapi_krb5.so.2") directly (no longer from the static constructor), which becomes the inner exception of the TypeInitializationException. The catch is now more specific with when (tie.InnerException is DllNotFoundException).
…on in GssInitializer, add when guard Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/9feee3d0-a8a2-4360-a981-593fe45e6f8b Co-authored-by: rzikm <32671551+rzikm@users.noreply.github.com>
Done in b04fdcb:
|
...mon/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.IsNtlmInstalled.cs
Outdated
Show resolved
Hide resolved
…tic constructor Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fd3637bc-49ab-45df-96df-8de0eaebab3b Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
|
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
There was a problem hiding this comment.
Pull request overview
Updates the Unix Negotiate/GSSAPI integration to avoid writing directly to stderr when the optional libgssapi_krb5.so.2 is missing, and instead rely on managed tracing + more descriptive exceptions to communicate the “not available” condition.
Changes:
- Native: stop printing
dlopen-failure diagnostics tostderrwhenlibgssapi_krb5.so.2is absent. - Common interop: change GSS initializer failure to throw
DllNotFoundException("libgssapi_krb5.so.2"). - System.Net.Security: add handling/logging for GSS “library not found” initialization failures and add tracing for native-availability checks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/native/libs/System.Net.Security.Native/pal_gssapi.c | Removes stderr logging on dlopen failure for optional GSS library. |
| src/libraries/Common/src/Interop/Unix/System.Net.Security.Native/Interop.NetSecurityNative.IsNtlmInstalled.cs | Changes GSS init failure to throw DllNotFoundException with library name. |
| src/libraries/System.Net.Security/src/System/Net/NegotiateAuthenticationPal.Unix.cs | Adds exception handling/logging for GSS “not installed” cases and logs availability-check failures via NetEventSource. |
| Exception logException = e is TypeInitializationException tie ? tie.InnerException ?? e : e; | ||
| if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, logException); |
There was a problem hiding this comment.
When logging TypeInitializationException, this only unwraps one level. In the common case here (NetSecurityNative cctor failing due to GssInitializer cctor), InnerException is another TypeInitializationException, so the logged exception won’t include the underlying DllNotFoundException directly. Consider unwrapping TypeInitializationException recursively so tracing shows the real root cause (and to keep behavior consistent with the Create() handling).
There was a problem hiding this comment.
It would be better to avoid depending on TypeInitializationException unwrapping instead.
| private const string GssApiLibraryName = "libgssapi_krb5.so.2"; | ||
|
|
||
| static GssInitializer() | ||
| { | ||
| if (EnsureGssInitialized() != 0) | ||
| { | ||
| throw new InvalidOperationException(); | ||
| throw new DllNotFoundException(GssApiLibraryName); | ||
| } |
There was a problem hiding this comment.
EnsureGssInitialized() returns -1 for both dlopen failures (libgssapi_krb5 missing) and dlsym failures (required symbol missing) in pal_gssapi.c. Throwing DllNotFoundException for any non-zero return conflates these cases and can cause callers to treat an unexpected symbol-missing scenario as a simple optional-library absence. Consider updating the native shim to return distinct error codes (e.g., library-not-found vs symbol-not-found) and throw a more accurate exception (e.g., EntryPointNotFoundException or InvalidOperationException) for the symbol-missing case.
| catch (TypeInitializationException tie) when (tie.InnerException is DllNotFoundException) | ||
| { | ||
| // GSSAPI native library (e.g. libgssapi_krb5) may not be available on the system | ||
| if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, tie.InnerException); | ||
| return new UnsupportedNegotiateAuthenticationPal(clientOptions); | ||
| } |
There was a problem hiding this comment.
The catch filter only matches when the thrown TypeInitializationException’s immediate InnerException is a DllNotFoundException. In this code path, Interop.NetSecurityNative’s type initializer calls GssInitializer.Initialize(), so failures typically become nested TypeInitializationExceptions (NetSecurityNative -> GssInitializer -> DllNotFoundException). As a result, this catch may not run and the exception can escape instead of falling back to Unsupported. Consider unwrapping TypeInitializationException(s) to the root cause (and logging/handling based on that).
| catch (TypeInitializationException tie) when (tie.InnerException is DllNotFoundException) | ||
| { | ||
| // GSSAPI native library (e.g. libgssapi_krb5) may not be available on the system | ||
| if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(null, tie.InnerException); | ||
| return new UnsupportedNegotiateAuthenticationPal(serverOptions); | ||
| } |
There was a problem hiding this comment.
Same issue as the client Create() overload: the filter only checks one InnerException level. Because NetSecurityNative’s static ctor invokes GssInitializer.Initialize(), missing libgssapi_krb5 typically results in nested TypeInitializationExceptions, so this catch may not trigger. Unwrap TypeInitializationException(s) to find the underlying DllNotFoundException and handle/log that.
🤖 Copilot Code Review — PR #126300Note This review was generated by Copilot. Holistic AssessmentMotivation: Justified. In chiseled/minimal containers without Approach: Good overall — remove noisy stderr at the source, use a more specific exception type ( Summary: Detailed Findings
|
When
libgssapi_krb5.so.2is absent (e.g. chiseled containers), the native shim wrote directly tostderrbefore returning a failure code, causing spurious error output even when the caller handled the absence gracefully.Changes
pal_gssapi.cfprintf(stderr, …)from thedlopenfailure path inensure_gss_shim_initialized()— the library is optional, so a missing library is silently handled.fprintf(stderr, …)for thedlsymfailure path (PER_FUNCTION_BLOCKmacro) — if the library is found but a required symbol is missing, that is unexpected and warrants a diagnostic message (consistent with the OpenSSL shim pattern).Interop.NetSecurityNative.IsNtlmInstalled.csGssInitializer's static constructor to throwDllNotFoundException("libgssapi_krb5.so.2")instead of a bareInvalidOperationExceptionwhen GSSAPI initialization fails.Initialize()remains a no-op hook, preserving the original structure while making the propagated exception type more descriptive.NegotiateAuthenticationPal.Unix.cscatch (TypeInitializationException tie) when (tie.InnerException is DllNotFoundException)to bothCreate(NegotiateAuthenticationClientOptions)andCreate(NegotiateAuthenticationServerOptions). The guard makes the handling specific to the GSSAPI library-not-found case;tie.InnerException(theDllNotFoundExceptionwith the library name) is passed toNetEventSource.Errorfor a more descriptive message.CheckHasSystemNetSecurityNative()similarly logs the inner exception of aTypeInitializationExceptionwhen available, for more descriptive tracing output.NetEventSource.Errorlogging inCheckHasSystemNetSecurityNative()when any of the caught availability-check exceptions fires.📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.