Enable NegotiateStream For Unix#6469
Conversation
| assert(outBuffer != nullptr); | ||
| // count refers to the length of the input message. That is, number of bytes of inputBytes | ||
| // starting at offset | ||
| // that need to be wrapped. |
There was a problem hiding this comment.
Nit: why the extra line wrap here?
| Debug.Assert(offset >= 0 && offset <= inputBytes.Length, "offset must be valid"); | ||
| Debug.Assert( count >=0 && count <= inputBytes.Length, "count must be valid"); | ||
| Debug.Assert(count + offset <= inputBytes.Length, "offset and count must be valid"); | ||
| Debug.Assert(count >=0 && count <= inputBytes.Length, "count must be valid"); |
There was a problem hiding this comment.
Nit: Missing space between >= and 0.
|
Aside from the one missing space, the new commit looks good. Pending changes I don't see reflected in the deferral issue:
Open questions:
I'm okay with those being deferred, if that's the plan; but the feedback shouldn't be lost. (And Steve and I had a few comments about improving the test code). Since it was pointed out to me that while Ubuntu 14.04 OuterLoop failed previously, 3 other distros ran; so the tests have been seen to pass (e.g. http://dotnet-ci.cloudapp.net/job/dotnet_corefx/job/outerloop_ubuntu15.10_debug_tst_prtest/3/testReport/System.Net.Security.Tests/KerberosTest/), so I'm not concerned that it hasn't been tested. As long as the other pending work/questions are tracked (or fixed) LGTM. |
Enabling NegotiateStream for Unix using native gssapi implementation. The native libraries used are mit-krb5 on linux and the built-in GSS.Framework for OSX. Also included are the tests which rely on configuring and deploying KDC on the host machine.
kapilash
force-pushed
the
negotiatestream
branch
from
ce252ba to
facff9e
Compare
These are all fixed.
Updated #7031 with these and other issues. I believe I did not miss any thing now. Thanks a lot @bartonjs , @stephentoub |
Enable NegotiateStream For Unix
| internal class NegotiationInfoClass | ||
| internal partial class NegotiationInfoClass | ||
| { | ||
| internal const string NTLM = "NTLM"; |
There was a problem hiding this comment.
What was the reason this got separated in two? I find it odd to have a partial class only to define 3 const strings. Shouldn't we create a new class for these strings instead if they can't live here?
13 participants
Problem
Enabling
System.Net.Security.NegotiateStreamfor Unix.Solution
This PR introduces the implementation of
NegotiateStreambased on native implementations of gssapiThe native implementations used are mit-krb5 on linux and the built-in GSS.Framework for OSX.
Also included are scripts to configure and deploy KDC on the host machine so as to run the associated tests.
cc: @stephentoub @bartonjs @CIPop @davidsh @vijaykota @shrutigarg