Conversation
ghost
added
area-System.Net.Security
community-contribution
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsOn macOS the gss_accept_sec_context/gss_init_sec_context APIs release the context handle when error occurs. The code didn't handle it properly and it would result in double-free and hard crash. Update the code to handle this situation properly. Fixes #71463
|
… the context handle when error occurs. The code didn't handle it properly and it would result in double-free and hard crash. Update the code to handle this situation properly.
filipnavara
force-pushed
the
gss-context-null-macos
branch
from
721a4a3 to
00b2786
Compare
| } | ||
| } | ||
|
|
||
| private static bool GssInitSecurityContext( |
There was a problem hiding this comment.
The GssInitSecurityContext and GssAcceptSecurityContext methods are folded into the caller. This made updating the handles substantially easier. I also removed the code that threw GssApiException only to catch it one method above in the stack and convert it to reported status code.
| : base(credential) | ||
| { | ||
| Debug.Assert((null != credential), "Null credential in SafeDeleteNegoContext"); | ||
| _context = new SafeGssContextHandle(); |
There was a problem hiding this comment.
Instead of trying to handle null values here it's easier to just always have a non-null handle.
There was a problem hiding this comment.
I assume we need to always allocate it anyway? #69527 is trying to avoid unnecessary allocations.
There was a problem hiding this comment.
Yes, it would always get allocated right after SafeDeleteNegoContext is created.
|
|
||
| public void SetGssContext(SafeGssContextHandle context) | ||
| { | ||
| Debug.Assert(context != null && !context.IsInvalid, "Invalid context passed to SafeDeleteNegoContext"); |
There was a problem hiding this comment.
We now explicitly allow invalid handles to reset the previous values so the Assert became useless. context == null is already checked by the nullability in the compiler.
|
I assume this is independent of #71373, right? |
Yep. Although it may create some merge conflict. |
src/libraries/Common/src/System/Net/Security/NegotiateStreamPal.Unix.cs
Outdated
Show resolved
Hide resolved
3 participants
On macOS the gss_accept_sec_context/gss_init_sec_context APIs release the context handle when error occurs. The code didn't handle it properly and it would result in double-free and hard crash. Update the code to handle this situation properly.
Fixes #71463